Skip to content

Fix issues found during the connection-reuse review - #36

Merged
jonathan-kosgei merged 7 commits into
masterfrom
fix-noticed-issues
Nov 3, 2026
Merged

jonathan-kosgei merged 7 commits into
masterfrom
fix-noticed-issues

Conversation

@jonathan-kosgei

@jonathan-kosgei jonathan-kosgei commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the items recorded under "Noticed, not changed" in #35: bulk() given a set, DotDict dot access to a list of scalars, the CLI batch --exclude crash, missing threat columns in the batch CSV output, a wrong flag in the README batch example, two live tests that compared a value with itself, and the CI workflow's Node 20 actions and 2022 Python pin. No version number changes and no breaking changes to the public API.

Changes

  • bulk() accepts a set of resources: the resources are sent as a JSON list, so a set no longer fails with IPDataException("... Object of type set is not JSON serializable"). (ac32091)
  • DotDict: dot access to a list of scalars (data.x when x == [1, 2, 3]) returns the list instead of raising TypeError; only dict elements are wrapped, so data.threat.blocklists[0].name works as before. (7ce7d14)
  • CLI: ipdata batch -e <field> works instead of raising UnboundLocalError; the remaining fields are requested in sorted order so that CSV columns are stable, and --format CSV --exclude <field> is accepted (CSV output needs either --fields or --exclude). (017c61e)
  • CLI: batch CSV output gains the threat_is_vpn and threat_scores columns, which were dropped from the rows before; the blocklist dates are part of each entry in the existing threat_blocklists column. (3504289)
  • README: the batch CSV example uses --format CSV (the command has no --output-format option) and the text names --exclude as the alternative to --fields. (924985c)
  • Live tests: test_bulk_v4 and test_bulk_v6 compare the returned IPs with the requested ones (case-insensitively for IPv6) instead of comparing each requested IP with itself. (9d42f29)
  • CI: actions/checkout@v7 and actions/setup-python@v7 (Node 24) replace the Node 20 @v4 and @v5; Python 3.14 joins the test matrix and the classifiers; the publish job uses Python 3.13 instead of the pinned 3.10.4 and installs build without --user. Its steps, tag gate and runner image are otherwise unchanged. (07ec54d)

Not changed

  • Per-call timeout for lookup() / bulk(): a new feature rather than a defect; the constructor timeout applies to every request, bulk lookups included.
  • The 100 hypothesis examples per live test: a cost/coverage trade-off on the live API quota for the maintainers to decide; the assertions themselves are fixed above.

Verification

  • python -m pytest -m "not api" (Python 3.13.18, package installed in editable mode, requests not installed): 94 passed, 4 deselected (87 passed, 4 deselected on master; the 7 new tests cover each behaviour change). The single warning is a DeprecationWarning raised by click_default_group when the CLI module is imported and predates this branch.
  • The same command under Python 3.9.25 and 3.14.6 (virtualenvs built from requirements.txt, package not installed): 94 passed, 4 deselected each.
  • python -m pytest -m api --collect-only: the four live tests still collect. They are not run here (they need IPDATA_API_KEY and run on pushes to master).
  • ast.parse(source, feature_version=(3, 9)) on every changed module: OK. The workflow parses as YAML with the expected matrix, action versions and publish Python version.
  • CI: run https://github.com/ipdata/python/actions/runs/41639769512 on 07ec54d (pull_request event): success. Test (Python 3.9), 3.10, 3.11, 3.12, 3.13 and 3.14 all passed python -m pytest -m "not api" on ubuntu-24.04 with actions/checkout@v7 and actions/setup-python@v7; Build and publish to PyPI was skipped on the pull request, as intended.

🤖 Generated with Claude Code

bulk() accepts a set of resources in its validation, but the set was
handed to the JSON encoder as-is, which cannot serialise it, so every
call with a set failed with "Object of type set is not JSON
serializable" wrapped in an IPDataException. The resources are now sent
as a list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
DotDict.__getattr__ wrapped every element of a list value in a DotDict,
so reading an attribute whose value is a list of scalars, for example
[1, 2, 3], raised TypeError. Only dict elements are wrapped now and the
other elements are returned unchanged. Lists of objects such as
threat.blocklists behave as before.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
"ipdata batch -e <field>" raised UnboundLocalError because the excluded
fields were subtracted from ipdata.valid_fields before the ipdata client
was assigned in the same function. The class attribute is used instead,
and the remaining fields are sorted so that CSV columns always come out
in the same order. The check that CSV output needs a field list now runs
after --exclude has been turned into one, so "--format CSV --exclude
<field>" works as well.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The list of threat members that the batch command expands into CSV
columns did not include is_vpn or scores, so threat_is_vpn and
threat_scores were silently dropped from CSV rows while the JSON output
had them. Both columns are written now, in the position the API returns
them. The blocklist dates need no column of their own: they are part of
each entry in the threat_blocklists column.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The example used --output-format, which the batch command does not
have; the option is --format. The sentence after it now also names
--exclude as the alternative to --fields for CSV output.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
test_bulk_v4 and test_bulk_v6 compared each requested IP with itself,
so they never checked the addresses the API returned. They now check
that the returned addresses are exactly the requested ones, compared
case-insensitively for IPv6.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…o Python 3.13

actions/checkout@v4 and actions/setup-python@v5 run on Node 20, which
GitHub has deprecated on its runners; both move to their current major
versions, which run on Node 24. Python 3.14 joins the test matrix and
the classifiers. The publish job no longer pins the 2022 patch release
3.10.4; it uses 3.13, a version the test matrix covers, and installs
build without --user.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jonathan-kosgei
jonathan-kosgei merged commit 642fcc1 into master Nov 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants