Repository navigation
chore: allow huggingface-hub 2.x - #1117
davanstrien wants to merge 3 commits into
Conversation
[Medium impact] The PR appears safe to merge; the previous finding is fixed and no new actionable issues remain. SummaryAllows
Reviews (3) · Last reviewed commit: "test: annotate noisy logger test paramet..." · Reviewed by Greptile |
|
Thanks for this! Supporting Hub 2.x while keeping 1.x compatibility is useful. We’ll confirm which release to target shortly. |
|
|
||
|
|
||
| _DEFAULT_NOISY_LOGGERS = ["httpx", "matplotlib", "mcp"] | ||
| _DEFAULT_NOISY_LOGGERS = ["httpx", "httpx2", "matplotlib", "mcp"] |
There was a problem hiding this comment.
Could we extend test_configure_logging_silences_default_noisy_loggers to cover httpx2 too? Parametrizing it over httpx and httpx2, and resetting each logger’s level before configuring logging, would protect against regressions and test-order effects. Non-blocking suggestion.
|
Could we add a patched |
Widen the huggingface-hub cap to <3 and upgrade the lock to 2.4.0. Quieten the httpx2 logger, which hub 2.x uses for requests. Update the CLI test for the new click 8.4 error message. Fixes NVIDIA-NeMo#1110 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Daniel van Strien <davanstrien@gmail.com>
Add httpx2>=2.12.0 to the workspace and e2e security constraints and regenerate constraints.txt. Hub 2.x allows httpx2>=2.0.0, and 2.12.0 is the lowest version that patches all published advisories. Parametrize the noisy logger test over httpx, httpx2 and matplotlib, and reset each logger's level first to avoid test-order effects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Daniel van Strien <davanstrien@gmail.com>
384ba4c to
d363fa7
Compare
|
Thanks, both suggestions done in d363fa7, and I rebased on main.
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Daniel van Strien <davanstrien@gmail.com>
📋 Summary
Widens the
huggingface-hubcap to<3and upgrades the lock to 2.4.0, so Data Designer can be installed alongside packages that need hub 2.x. Data Designer's own Hub calls needed no changes. The only source change is one logger name.🔗 Related Issue
Fixes #1110
🔄 Changes
huggingface-hub>=1.0.1,<3indata-designeranddata-designer-engine<8.4) and datasets to 5.1.0 (5.0.1 caps hub at<2), and adds httpx2, httpcore2, httpx2-jsfetch and truststorehttpx2to_DEFAULT_NOISY_LOGGERS. Hub 2.x sends requests throughhttpx2, so without this every Hub request logs at INFO, including signed CDN URLstest_app_rejects_unknown_options_before_script_separatorfor click 8.4A correction to the issue: the test doesn't fail because of line wrapping. Click 8.4 changed the wording from
No such option: --xtoNo such option '--x'., so normalising whitespace wouldn't fix it. The test now checksNo such optionand--num-recrodsseparately, which passes on click 8.3.3 and 8.4.2.🧪 Testing
make testpasses (run aspytestper package in a locked environment on macOS, Python 3.12: config 830 passed, engine 2862 passed, interface 1309 passed and 11 skipped; thehttpx2line came after this run, and the 37 logging tests were re-run after it)I ran two live smoke tests on hub 2.4.0 against temporary private dataset repos, deleted afterwards. The scripts aren't part of this PR.
Small run:
push_to_hub_from_folderto a new repo, then again to the same repoLarger run (242 MB: 3 parquet batches of 40,000 rows, 20 images, 1 processor output):
push_to_hub_from_folderuploaded every file in 18 s, and a downloaded batch matched by SHA-256HuggingFaceSeedReaderreturned the expected columns and 120,000 rowsHfFileSystemtook 6 s, and the row count and checksums matchedHTTP Requestlines appeared in the log afterconfigure_logging()Not tested: a live run on hub 1.x (44 mocked Hugging Face integration tests pass on 1.0.1), and custom CA certificates (
SSL_CERT_FILE) under the new HTTP stack.Notes for review
>=1.6, so it's easy to hold back if you prefer.typer>=0.12looks too low, separately from this PR: typer 0.15.2--helpraisesTypeErroron click 8.3.3 as well as 8.4.2, and 0.16.0 works on 8.4.2. I left it for a follow-up.data-designertest suite rewrites the repo'suv.lockusing the local user's uv config. This also looks separate from this PR, and I haven't traced it to a specific test (the plugin install tests are my first guess). It's worth knowing if you have a custom index orexclude-newerin your uv config.✅ Checklist
🤖 Generated with Claude Code