Skip to content

fix(poetry): keep /simple suffix on the resolution index URL - #536

Open
nicktr202 wants to merge 2 commits into
jfrog:mainfrom
nicktr202:fix/poetry-resolve-index-url-simple-suffix
Open

fix(poetry): keep /simple suffix on the resolution index URL#536
nicktr202 wants to merge 2 commits into
jfrog:mainfrom
nicktr202:fix/poetry-resolve-index-url-simple-suffix

Conversation

@nicktr202

@nicktr202 nicktr202 commented Aug 21, 2026

Copy link
Copy Markdown

Description

SetPypiRepoUrlWithCredentials strips the /simple suffix from the Artifactory URL before choosing the publish branch, so resolution commands (install, add, update, lock) are configured with the truncated URL too:

baseUrl := rtUrl.Scheme + "://" + rtUrl.Host + rtUrl.Path
publishUrl := strings.TrimSuffix(baseUrl, "/simple")   // applied unconditionally
publishUrl = strings.TrimSuffix(publishUrl, "/")

if pc.commandName == "publish" {
    return RunPoetryConfig(publishUrl, ...)
}
return ConfigPoetryRepo(publishUrl, ...)               // resolution gets the truncated URL

ConfigPoetryRepo writes that URL into [[tool.poetry.source]] in pyproject.toml. Poetry then queries .../api/pypi/<repo> as a PEP 503 index, which is not one, and reports every dependency as unresolvable:

[Info] Added tool.poetry.source name:"my-repo" url:"https://my.jfrog.io/artifactory/api/pypi/my-repo"
[Info] Running Poetry update
Because my-project depends on boto3 (>=1.42.24,<2.0.0) which doesn't match any versions, version solving failed.
[Error] Poetry config command failed with: exit status 1

The two URLs are genuinely different endpoints and the Poetry documentation is explicit about it: a package source must carry the trailing /simple/ ("Note the trailing /simple/. This is important when configuring PEP 503 compliant package sources"), and it notes that this "is not the same configuration used when publishing a package".

Introduced in 98f7b22 (PR #316). First released in JFrog CLI 2.87.0; 2.86.0 is the last good version.

Fix

The URL selection is extracted into poetryRepoUrl, which is applied after the command is known:

  • publish → upload endpoint, /simple stripped (unchanged behaviour)
  • every other command → PEP 503 index, /simple preserved

Why the existing test did not catch this

TestSetPypiRepoUrlWithCredentials_URLTransformation re-implemented the production logic inside the test body instead of calling it, and only ever asserted the publish case:

publishUrl := strings.TrimSuffix(baseUrl, "/simple")
assert.NotContains(t, publishUrl, "/simple", "URL should not contain /simple")

So it validated a copy of the code, never the resolution path, and would have stayed green after any regression in SetPypiRepoUrlWithCredentials. The test now calls poetryRepoUrl directly and asserts both outcomes: the publish URL without /simple, and the resolution URL with /simple preserved for install, add, update, lock and the empty command name.

Related issues

Tests

  • go build ./artifactory/commands/python/... — OK
  • go vet ./artifactory/commands/python/... — OK
  • go test ./artifactory/commands/python/... — OK

Summary by CodeRabbit

  • Bug Fixes
    • Improved Poetry repository URL handling for publishing and package installation.
    • Publishing now uses the repository’s base URL, while package resolution continues using the /simple index endpoint.
    • Preserved correct URL formatting across supported Poetry commands.

SetPypiRepoUrlWithCredentials stripped the /simple suffix from the
Artifactory URL before selecting the publish branch, so resolution
commands were also configured with the truncated URL. Poetry then wrote
.../api/pypi/<repo> into [[tool.poetry.source]] and queried it as a
PEP 503 index, which fails with "doesn't match any versions".

Extract the URL selection into poetryRepoUrl: publish keeps the upload
endpoint (no /simple), every other command keeps the PEP 503 index.

TestSetPypiRepoUrlWithCredentials_URLTransformation duplicated the
production logic instead of calling it, so it could not catch this.
It now calls poetryRepoUrl and asserts both the publish and the
resolution URL.

Fixes jfrog#381
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@nicktr202, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Limit details: You’ve used the included review currently available.

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?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f51c31eb-d871-406b-b1f3-3dfc664e6c61

📥 Commits

Reviewing files that changed from the base of the PR and between 885486a and 50231cc.

📒 Files selected for processing (2)
  • artifactory/commands/python/poetry.go
  • artifactory/commands/python/poetry_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e6780cce-9852-42d3-9099-90a43e7754f1

📥 Commits

Reviewing files that changed from the base of the PR and between a590327 and 885486a.

📒 Files selected for processing (2)
  • artifactory/commands/python/poetry.go
  • artifactory/commands/python/poetry_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Poetry repository URL handling now distinguishes publish URLs from dependency-resolution URLs. Publish commands remove /simple, while install and other resolution commands retain it. Tests cover both URL forms across supported repository configurations.

Changes

Poetry repository URL handling

Layer / File(s) Summary
Command-specific URL normalization
artifactory/commands/python/poetry.go
Adds poetryRepoUrl and uses it to preserve /simple for resolution commands and remove it for publishing.
Publish and resolve URL tests
artifactory/commands/python/poetry_test.go
Tests publish and resolve URLs for standard, alternate-repository, token, and trailing-slash configurations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 88548

The change preserves the required /simple suffix for dependency resolution while keeping the publish endpoint unchanged, and no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: reshmifrog, agrasth

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change restores /simple for resolution commands and keeps publish URLs separate, satisfying issues #381 and #3306.
Out of Scope Changes check ✅ Passed All code and test changes directly support the Poetry repository URL fix and its validation.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preserving the /simple suffix for Poetry resolution URLs.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nicktr202

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

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.

poetry install source url broken Poetry version solving fails when running jf poetry install

1 participant