Skip to content

Commit ee592e2

Browse files
committed
round one Claude code review
1 parent f929d9e commit ee592e2

22 files changed

Lines changed: 224 additions & 112 deletions

CLAUDE_TODO

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
3+
# TLD regex database compiled eagerly at import
4+
whoisdomain/helpers.py · lines 99–100
5+
6+
Importing whoisdomain forces processing all 3,000+ lines of ZZ immediately. Consumers who only need one TLD pay the cost of all of them. Make MY_TLD_INFO lazy — initialize on first call to get_TLD_RE() or filterTldToSupportedPattern(). Bonus: significantly faster python -c "import whoisdomain", which matters for CLI tools and lambdas.
7+
8+
---
9+
10+
# ParameterContext is a JSON-parsed config rather than a dataclass
11+
whoisdomain/context/parameterContext.py
12+
13+
A 150-line JSON literal is parsed at import to define ~25 parameters, then __getattr__ returns Any for every read. mypy can't help you, IDE autocomplete can't help your users, and the validation code (~80 lines) duplicates what @dataclass(slots=True) + __post_init__ would give you for free. Sketch:
14+
15+
@dataclass(slots=True)
16+
class ParameterContext:
17+
cmd: str = "whois"
18+
timeout: float = 30.0
19+
cache_age: int = 60 * 60 * 48
20+
verbose: bool = False
21+
rdap_only: bool = False
22+
# ... etc
23+
def to_json(self) -> str: return json.dumps(asdict(self))
24+
@classmethod
25+
def from_json(cls, s: str) -> "ParameterContext":
26+
return cls(**json.loads(s))
27+
28+
---
29+
30+
# Module-level mutable globals everywhere
31+
helpers.py:99, doWhoisCommand.py:20, lastWhois.py:21, main.py:550
32+
33+
MY_TLD_INFO, CACHE_STUB, LastWhois, and a 14-name global declaration in main(). This makes the package effectively a singleton — concurrent users (e.g. an async webapp) will fight over state, and tests can't run in parallel. Move shared state onto a Whois client class that callers instantiate, and keep the module-level helpers as thin wrappers around a default instance for backwards compatibility.
34+
35+
---
36+
37+
# Quick nits
38+
is True / is False comparisons throughout — just use if x: / if not x:
39+
~40 sites

Makefile.tests

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ TEST_OPTIONS_ALL = \
1212
# --------------------------------------------------
1313
# Tests
1414
# --------------------------------------------------
15-
# test: prep test1 test2 test3 test4
16-
test: prep test2
15+
test: prep test1 test2 test3 test4
16+
# test: prep test2
1717

1818
prep:
1919
mkdir -p tmp

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ and `make suggest`.
9999

100100
## in progress
101101

102+
103+
## 2.20260522.1
104+
102105
- switch to minimal version 3.10
103106
- update gitgub-action lint (mypy) to use `setup-python@v6` and `checkout@v6`
104107
- start working on v2 with rdap first
105-
106-
## 2.20260522.1

analizer/vtmp/bin/normalizer

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#!/home/mboot/DEV/00-github.com/mboot-github/WhoisDomain/analizer/vtmp/bin/python3.10
2-
# -*- coding: utf-8 -*-
3-
import re
42
import sys
53
from charset_normalizer.cli import cli_detect
64
if __name__ == '__main__':
7-
sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
5+
sys.argv[0] = sys.argv[0].removesuffix('.exe')
86
sys.exit(cli_detect())

analizer/vtmp/bin/pip

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#!/home/mboot/DEV/00-github.com/mboot-github/WhoisDomain/analizer/vtmp/bin/python3.10
2-
# -*- coding: utf-8 -*-
3-
import re
42
import sys
53
from pip._internal.cli.main import main
64
if __name__ == '__main__':
7-
sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
5+
sys.argv[0] = sys.argv[0].removesuffix('.exe')
86
sys.exit(main())

analizer/vtmp/bin/pip3

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#!/home/mboot/DEV/00-github.com/mboot-github/WhoisDomain/analizer/vtmp/bin/python3.10
2-
# -*- coding: utf-8 -*-
3-
import re
42
import sys
53
from pip._internal.cli.main import main
64
if __name__ == '__main__':
7-
sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
5+
sys.argv[0] = sys.argv[0].removesuffix('.exe')
86
sys.exit(main())

analizer/vtmp/bin/pip3.10

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#!/home/mboot/DEV/00-github.com/mboot-github/WhoisDomain/analizer/vtmp/bin/python3.10
2-
# -*- coding: utf-8 -*-
3-
import re
42
import sys
53
from pip._internal.cli.main import main
64
if __name__ == '__main__':
7-
sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
5+
sys.argv[0] = sys.argv[0].removesuffix('.exe')
86
sys.exit(main())

analizer/vtmp/bin/url-normalize

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
#!/home/mboot/DEV/00-github.com/mboot-github/WhoisDomain/analizer/vtmp/bin/python3.10
2-
# -*- coding: utf-8 -*-
3-
import re
42
import sys
53
from url_normalize.cli import main
64
if __name__ == '__main__':
7-
sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
5+
sys.argv[0] = sys.argv[0].removesuffix('.exe')
86
sys.exit(main())

whoisdomain/__init__.py

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
All public data is vizible via the __all__ List
1010
"""
1111

12-
import gc
1312
import logging
1413
import os
1514
import sys
@@ -67,9 +66,6 @@
6766
except ImportError as e:
6867
_ = e
6968

70-
if HAS_REDIS:
71-
from .cache.redisCache import RedisCache
72-
7369
WHOISDOMAIN: str = ""
7470
if os.getenv("WHOISDOMAIN"):
7571
WHOISDOMAIN = str(os.getenv("WHOISDOMAIN"))
@@ -88,6 +84,7 @@
8884
except ImportError as e:
8985
_ = e # ignore any error
9086

87+
9188
__all__ = [
9289
"VERSION",
9390
"ZZ",
@@ -124,6 +121,11 @@
124121
"validTlds",
125122
]
126123

124+
if HAS_REDIS:
125+
from .cache.redisCache import RedisCache
126+
127+
__all__ += ["RedisCache"]
128+
127129

128130
def _result2dict(
129131
func: Any,
@@ -195,13 +197,6 @@ def q2(
195197
domain: str,
196198
pc: ParameterContext,
197199
) -> Domain | None:
198-
if pc.verbose is True:
199-
logging.basicConfig(level="DEBUG")
200-
201-
gc.collect(0)
202-
gc.collect(1)
203-
gc.collect(2)
204-
205200
dc = DataContext(
206201
domain=domain,
207202
hasLibTld=TLD_LIB_PRESENT,
@@ -249,20 +244,7 @@ def q2(
249244
parser=parser,
250245
)
251246

252-
result = pwdr.processRequest()
253-
254-
del pwdr
255-
del dom
256-
del wci
257-
del parser
258-
del dc
259-
del pc
260-
261-
gc.collect(0)
262-
gc.collect(1)
263-
gc.collect(2)
264-
265-
return result
247+
return pwdr.processRequest()
266248

267249

268250
def query(
@@ -298,10 +280,7 @@ def query(
298280
# whoisOnly: bool = false,
299281
) -> Domain | None:
300282
assert isinstance(domain, str), Exception("`domain` - must be <str>")
301-
302-
if verbose is True:
303-
logging.basicConfig(level="DEBUG")
304-
283+
_ = verbose # ha_ck
305284
if pc is None:
306285
pc = ParameterContext(**kwargs)
307286

@@ -313,3 +292,21 @@ def query(
313292

314293
# Add get function to support return result in dictionary form
315294
get = _result2dict(query)
295+
296+
# CLAUDE: logging changes
297+
# Calling logging.basicConfig(level="DEBUG") inside library code mutates the application's root logger
298+
# and is widely considered bad library citizenship.
299+
# If the caller's app uses logging, you've just overridden their config.
300+
# Use a per-package logger and let the caller configure the level:
301+
# logging.getLogger("whoisdomain").setLevel("DEBUG"),
302+
# or document a setup_logging(verbose=) helper that callers can opt into.
303+
304+
# CLAUDE: memoryleak:
305+
# Three calls — gc.collect(0); gc.collect(1); gc.collect(2) — once on entry and once on exit.
306+
# gc.collect(2) already does generations 0 and 1, so the other two are redundant.
307+
# More importantly, manual GC in library code imposes work on callers who don't need it
308+
# (Python's GC is generational and usually correct without help).
309+
# The del spam right above isn't doing anything either
310+
# — locals are about to go out of scope.
311+
# If you're chasing a real leak, profile it with tracemalloc;
312+
# otherwise this is cargo-culted overhead.

whoisdomain/context/dataContext.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,6 @@ def __init__(
2929
self.whoisStr: str = "" # the result string from whois cli after clean
3030

3131
self.data: dict[str, Any] = {} # the data we need to build the domain object
32-
self.exeptionStr: str | None = None # if we handle exceptions as result string instead of throw
32+
self.exceptionStr: str | None = None # if we handle exceptions as result string instead of throw
3333
self.thisTld: dict[str, Any] = {} # the dict of regex and info as defined by ZZ and parsed by TldInfo
3434
self.servers: list[str] = [] # extract whois servers from the whois output (may need --verbose on rfc1036/whois)

0 commit comments

Comments
 (0)