Skip to content

Replaced the IPS_TO_SKIP regex with the upstream list of excluded IP addresses - #108

Merged
CodyCBakerPhD merged 12 commits into
mainfrom
excluded-ips-list
Oct 28, 2026
Merged

CodyCBakerPhD merged 12 commits into
mainfrom
excluded-ips-list

Conversation

@CodyCBakerPhD

@CodyCBakerPhD CodyCBakerPhD commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This follows dandi/s3-log-extraction#321, released in s3-log-extraction 1.14.1. That change makes the upstream ~/.s3-log-extraction/excluded_ips.txt the only list of excluded IP addresses and applies it at extraction too. The format is one address per line, with # comments.

  • Removed IPS_TO_SKIP and IPS_TO_SKIP_REGEX entirely. An unset or empty regex matched every IP, which was a trap. An empty list now excludes nothing.
  • DandiRemoteS3LogAccessExtractor(*, cache_directory=None, excluded_ips=None) forwards excluded_ips to the upstream extractor, defaulting to the upstream get_excluded_ips(). The constructor is now keyword-only, since it has two parameters. The only caller (the CLI) already passed keywords.
  • _dandi_extraction.awk reads the upstream EXCLUDED_IPS awk env entry (space-separated) into an array and does if (ip in EXCLUDED_IPS) next. The lines are the same as in the upstream generic script.
  • No new CLI option, so the file stays the one place to list addresses. The CLI is unchanged from main.
  • Raised the pin to s3_log_extraction>=1.14.1, the first published release carrying the change (1.13.0 was never published), and bumped the version to 1.7.0.
  • The README replaces the IPS_TO_SKIP section with the file, and notes that removing an address needs a fresh extraction to bring its requests back.

⚠️ Migration before deploying

The old IPS_TO_SKIP was a regex, so it may match ranges or patterns. Before running this version, convert it into explicit addresses, one per line, in ~/.s3-log-extraction/excluded_ips.txt. Patterns and CIDR networks are refused with an error. Any requester the regex matched but the list leaves out will start being extracted. The runner side is brain-bbqs-runners/dandi-usage-runner#12.

Testing

  • Against the published s3-log-extraction 1.14.1 from PyPI: pytest -m "not remote" gave 124 passed and 1 deselected. The deselected test is test_dandi_summaries.py::test_dandiset_summaries, which needs api.dandiarchive.org. The sandbox blocks that host, and the test fails the same way on main there.
  • A new ai_generated test runs the DANDI awk script end to end. It checks that only the exact excluded address is dropped and that a neighboring address is kept.
  • The extractor init test is parametrized over default, empty, and explicit excluded_ips.
  • pre-commit run --all-files is clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3

DandiRemoteS3LogAccessExtractor now takes excluded_ips, defaulting to the
upstream s3_log_extraction.config.get_excluded_ips(), and the DANDI awk script
drops exactly those addresses. Added --exclude to extract and update summaries,
and raised the upstream pin to >=1.12.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
@CodyCBakerPhD CodyCBakerPhD self-assigned this Oct 26, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3

CodyCBakerPhD commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

CI on this PR was expected to fail at dependency install until the upstream release was on PyPI. Resolved: s3-log-extraction 1.14.1 is now published, and the pin is raised to >=1.14.1, the first published release carrying dandi/s3-log-extraction#321. 1.13.0 was never published.


Generated by Claude Code

Follows the upstream change that makes S3_LOG_EXTRACTION_EXCLUDED_IPS in
~/.s3-log-extraction/.env the single source, replacing excluded_ips.txt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Follows the upstream change that accepts a double-quoted multi-line value
with per-line comments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Follows the upstream change that drops the dotenv entry and --exclude.
The --exclude option is removed from extract and update summaries, the docs
point at ~/.s3-log-extraction/excluded_ips.txt, and the upstream pin is raised
to >=1.13.0, the version that carries the change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
The upstream EXCLUDED_IPS_FILE_PATH is no longer public.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
@CodyCBakerPhD
CodyCBakerPhD marked this pull request as ready for review October 28, 2026 04:54
CodyCBakerPhD and others added 3 commits October 28, 2026 01:40
1.13.0 was never published, and 1.14.1 is the first release carrying the
excluded_ips.txt extraction change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
Brings in the extract --check flag of #107, which already raised the upstream
pin to >=1.14.1. Both changelog entries stay under Upcoming, and the version
stays at 1.7.0, this pull request's single bump above main's 1.6.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LWo8Rh1Ceh8UAWkJCdHou3
@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.10%. Comparing base (2e2fd34) to head (3d93fc5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #108       +/-   ##
===========================================
+ Coverage   67.05%   85.10%   +18.04%     
===========================================
  Files           4        9        +5     
  Lines         170      658      +488     
===========================================
+ Hits          114      560      +446     
- Misses         56       98       +42     
Flag Coverage Δ
unittests 85.10% <100.00%> (+18.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...xtractors/_dandi_remote_s3_log_access_extractor.py 100.00% <100.00%> (ø)
...traction/summarize/_generate_dandiset_summaries.py 100.00% <ø> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CodyCBakerPhD
CodyCBakerPhD merged commit fa38d75 into main Oct 28, 2026
7 checks passed
@CodyCBakerPhD
CodyCBakerPhD deleted the excluded-ips-list branch October 28, 2026 05:50
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