Conversation
Resolve mvnrepository links by checking common repository hosts in popularity order instead of hard-falling back to Google Maven only.
Keep the requested GAV when any known host has it, and only fall back to the highest-priority repository latest when no exact version matches.
Keep leftover git fail text out of success results so callers see a clean message.
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMaven resolution now probes configured repositories for versions, metadata, source JARs, and artifact bases. Download fallback handling now clears stale errors and omits empty wget errors. ChangesMaven repository resolution
Fallback download error reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant get_download_location_for_maven
participant MavenRepository
CLI->>get_download_location_for_maven: request Maven download
get_download_location_for_maven->>MavenRepository: probe version, metadata, or source JAR
MavenRepository-->>get_download_location_for_maven: return repository URL
get_download_location_for_maven-->>CLI: return resolved download location
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/fosslight_util/_get_downloadable_url.py (2)
924-945: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider
defusedxmlfor remote XML.
ET.fromstringparses content from remote hosts.defusedxml.ElementTreeblocks entity-expansion attacks and is the recommended parser for untrusted input. Ruff flags this as S314.The risk is limited here, because the hosts are a fixed allowlist. Treat this as hardening.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_util/_get_downloadable_url.py` around lines 924 - 945, Update _maven_latest_version_from_repo to parse remote Maven metadata with defusedxml.ElementTree instead of the standard ET parser. Preserve the existing metadata extraction and exception handling behavior while replacing the ET.fromstring call and corresponding import with the hardened parser.Source: Linters/SAST tools
967-988: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the directory listing when the host does not serve listings.
_probe_maven_sources_jarcalls_maven_sources_from_directoryfor every repository, including hosts that return an HTML error page or no listing. That adds one blockingurlopenper repository on top of the three HEAD probes. Consider running the listing step only after_maven_http_ok(version_dir)confirms the version directory exists.♻️ Proposed refactor
def _probe_maven_sources_jar(group_path: str, artifact_id: str, version: str) -> str: for repo_base in MAVEN_REPOSITORY_BASES: version_dir = f"{repo_base}/{group_path}/{artifact_id}/{version}" for classifier in MAVEN_SOURCE_CLASSIFIERS: sources_url = f"{version_dir}/{artifact_id}-{version}-{classifier}.jar" if _maven_http_ok(sources_url): logger.info(f"Maven sources found: {sources_url}") return sources_url + if not _maven_http_ok(f"{version_dir}/"): + continue listed = _maven_sources_from_directory(version_dir, artifact_id, version)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_util/_get_downloadable_url.py` around lines 967 - 988, Update _probe_maven_sources_jar so _maven_sources_from_directory is called only when _maven_http_ok(version_dir) confirms the Maven version directory exists. Preserve the existing classifier checks, listing result handling, repository iteration, and empty-string fallback.tests/test_download_maven.py (2)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWiden the fake signature to accept any arguments.
fake_getaccepts onlyurlandtimeout. Ifversion_existslater adds a header or parameter argument, this test fails with aTypeErrorinstead of a clear assertion. The other fakes in this file already use*_args, **_kwargs.♻️ Proposed refactor
- def fake_get(url, timeout=5): + def fake_get(url, *_args, **_kwargs): assert "deps.dev" in url return _FakeResponse(200, {"versions": [{"versionKey": {"version": "6.1.14"}}]})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_download_maven.py` around lines 27 - 31, Update the fake_get test helper to accept arbitrary positional and keyword arguments while preserving its URL assertion and response behavior, matching the flexible signatures used by the other fakes in the file.
126-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the no-sources failure path.
This PR changes
get_download_location_for_mavento raise an error when no configured repository contains the artifact. The previous code fell back to Google Maven. That behavior change has no test. Add a case where_probe_maven_sources_jarreturns""and assert the function returns(False, '').💚 Proposed test
def test_get_download_location_for_maven_returns_false_when_no_sources(monkeypatch): monkeypatch.setattr( downloadable_url, "_probe_maven_sources_jar", lambda *_args, **_kwargs: "", ) ok, url = downloadable_url.get_download_location_for_maven( "mvnrepository.com/artifact/io.confluent/kafka-avro-serializer/8.2.1" ) assert ok is False assert url == "" def test_get_download_location_for_maven_returns_artifact_base_without_version(monkeypatch): base = "https://repo1.maven.org/maven2/org/springframework/spring-core" monkeypatch.setattr( downloadable_url, "_probe_maven_artifact_base", lambda group_path, artifact_id: base, ) ok, url = downloadable_url.get_download_location_for_maven( "mvnrepository.com/artifact/org.springframework/spring-core" ) assert ok is True assert url == base🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_download_maven.py` around lines 126 - 144, Add a test alongside test_get_download_location_for_maven_uses_candidate_sources that stubs downloadable_url._probe_maven_sources_jar to return an empty string, invokes get_download_location_for_maven with the same versioned artifact, and asserts the result is (False, "").
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_util/_get_downloadable_url.py`:
- Around line 908-921: The Maven version probe in _maven_version_available
currently makes too many slow requests; reduce each repository check to the .pom
and, optionally, the version directory, and lower the per-probe timeout while
preserving exact-version detection. At
src/fosslight_util/_get_downloadable_url.py:908-921, update
_maven_version_available accordingly; at
src/fosslight_util/_get_downloadable_url.py:314-332, verify version_exists has
acceptable worst-case batch latency after the reduction and add a per-artifact
result cache if needed.
- Around line 590-593: Add an explicit timeout to the deps.dev fallback request
in the find_version branch of _get_downloadable_url, matching the timeout
convention used by other requests.get calls in the module.
- Around line 948-964: Update _maven_sources_from_directory to pass the
established probe timeout to urlopen, and import and use urllib.parse.urljoin
when resolving both preferred and fallback hrefs against the directory URL.
Preserve the existing source-jar selection and empty-string error behavior while
ensuring relative, root-relative, and absolute links produce valid URLs.
- Around line 1015-1020: Update get_download_location_for_maven() so the
artifact_base fallback from _probe_maven_artifact_base is not returned as a
downloadable URL when latest-version resolution fails. Return False for
unresolved versions, or resolve a concrete version and return its sources JAR
URL instead; preserve successful resolved-version behavior.
In `@src/fosslight_util/download.py`:
- Around line 358-363: Update the message-selection logic in the download error
handling so msg_wget is checked before msg. When both are present after a failed
Git clone and wget fallback, set the result exclusively to wget fail:
{wget_message}; preserve the existing git and RubyGems handling for cases
without a wget failure.
---
Nitpick comments:
In `@src/fosslight_util/_get_downloadable_url.py`:
- Around line 924-945: Update _maven_latest_version_from_repo to parse remote
Maven metadata with defusedxml.ElementTree instead of the standard ET parser.
Preserve the existing metadata extraction and exception handling behavior while
replacing the ET.fromstring call and corresponding import with the hardened
parser.
- Around line 967-988: Update _probe_maven_sources_jar so
_maven_sources_from_directory is called only when _maven_http_ok(version_dir)
confirms the Maven version directory exists. Preserve the existing classifier
checks, listing result handling, repository iteration, and empty-string
fallback.
In `@tests/test_download_maven.py`:
- Around line 27-31: Update the fake_get test helper to accept arbitrary
positional and keyword arguments while preserving its URL assertion and response
behavior, matching the flexible signatures used by the other fakes in the file.
- Around line 126-144: Add a test alongside
test_get_download_location_for_maven_uses_candidate_sources that stubs
downloadable_url._probe_maven_sources_jar to return an empty string, invokes
get_download_location_for_maven with the same versioned artifact, and asserts
the result is (False, "").
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 436d2757-3d60-4bf8-9237-4a0356e11da2
📒 Files selected for processing (4)
src/fosslight_util/_get_downloadable_url.pysrc/fosslight_util/download.pytests/test_download_maven.pytests/test_download_version_hint.py
Prefer groupId host hints, shorten HTTP timeouts, and cache sources probes so downloads do not stall on dead mirrors.
Prevent hanging when the deps.dev fallback request stalls.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests