Skip to content

Poetry native support improvement#316

Merged
agrasth merged 3 commits intojfrog:mainfrom
agrasth:poetry-native-mode-enhancements
Dec 9, 2025
Merged

Poetry native support improvement#316
agrasth merged 3 commits intojfrog:mainfrom
agrasth:poetry-native-mode-enhancements

Conversation

@agrasth
Copy link
Collaborator

@agrasth agrasth commented Nov 30, 2025

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • Appropriate label is added to auto generate release notes.
  • I used gofmt for formatting the code before submitting the pull request.
  • PR description is clear and concise, and it includes the proposed solution/fix.

Title

Fix Poetry publish URL and improve publish command handling

Description

What

  1. Fixed jf setup poetry to configure the correct URL for publishing (without /simple suffix)
  2. Added special handling for publish command to avoid modifying pyproject.toml

Why

Issue 1: Incorrect Publish URL

The GetPypiRepoUrlWithCredentials function always appends /simple to URLs:

  • Resolution URL: https://server/artifactory/api/pypi/repo/simple (correct)
  • Publish URL: https://server/artifactory/api/pypi/repo/ (needs NO /simple)

Using the resolution URL for publishing causes 415 Unsupported Media Type errors.

Issue 2: Publish Command Side Effects

The publish command was calling ConfigPoetryRepo() which:

  • Modifies pyproject.toml
  • Runs poetry update

This is unnecessary and unwanted for publishing.

Changes

artifactory/commands/setup/setup.go:

func (sc *SetupCommand) configurePoetry() error {
    repoUrl, username, password, err := python.GetPypiRepoUrlWithCredentials(...)
    // Strip "simple" from URL for publishing support (same as Twine)
    publishUrl := strings.TrimSuffix(repoUrl.String(), "simple")
    return python.RunPoetryConfig(publishUrl, username, password, sc.repoName)
}

artifactory/commands/python/poetry.go:

func (pc *PoetryCommand) SetPypiRepoUrlWithCredentials() error {
    // ...
    if password != "" {
        // For publish commands, only configure Poetry TOML, don't modify pyproject.toml
        if pc.commandName == "publish" {
            return RunPoetryConfig(rtUrl.Scheme+"://"+rtUrl.Host+rtUrl.Path, username, password, pc.repository)
        }
        // For install/other commands, configure Poetry TOML and add to pyproject.toml
        return ConfigPoetryRepo(rtUrl.Scheme+"://"+rtUrl.Host+rtUrl.Path, username, password, pc.repository)
    }
    return nil
}

Related PRs

return pc
}

func (pc *PoetryCommand) SetPypiRepoUrlWithCredentials() error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add unit tests for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes doing

// For publish commands, only configure Poetry TOML, don't modify pyproject.toml
if pc.commandName == "publish" {
return RunPoetryConfig(
rtUrl.Scheme+"://"+rtUrl.Host+rtUrl.Path,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does rtUrl give trailing /

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, rtUrl does NOT give a trailing / after our fix:

publishUrl := strings.TrimSuffix(baseUrl, "/simple")  // Removes /simplepublishUrl = strings.TrimSuffix(publishUrl, "/")      // Removes any trailing /

Result:

  • Before: https://server/artifactory/api/pypi/repo/simple
  • After: https://server/artifactory/api/pypi/repo (no trailing slash)

// Strip "simple" from URL for publishing support (same as Twine)
// Resolution URL (with /simple) should be configured in pyproject.toml
// Publishing URL (without /simple) is configured in Poetry config
publishUrl := strings.TrimSuffix(repoUrl.String(), "simple")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change required? I am dont think this is part of poetry command/config flow. Could you please verify?

@agrasth agrasth added the improvement Automatically generated release notes label Dec 1, 2025
- Fix URL construction to properly strip /simple suffix and trailing slashes for publishing
- Ensure publish URL doesn't contain /simple (used for resolution only)
- Add comprehensive unit tests for URL transformation logic
- Update setup command to strip trailing slashes consistently
- Update go.mod with latest build-info-go dependency

Testing:
- Poetry install with build info collection works correctly
- Poetry publish with build info uploads artifacts and tracks in build info
- Multiple repositories configuration correctly selects primary repo
- URLs configured correctly: pyproject.toml has /simple, poetry config doesn't
@agrasth agrasth force-pushed the poetry-native-mode-enhancements branch from 66627e6 to 104b4d0 Compare December 9, 2025 12:00
@agrasth agrasth added the safe to test Approve running integration tests on a pull request label Dec 9, 2025
@agrasth agrasth merged commit 98f7b22 into jfrog:main Dec 9, 2025
11 of 12 checks passed
nitinp19 pushed a commit to nitinp19/jfrog-cli-artifactory that referenced this pull request Dec 11, 2025
// Strip "simple" and trailing slash from URL for publishing support (same as Twine)
// Resolution URL (with /simple) should be configured in pyproject.toml
// Publishing URL (without /simple) is configured in Poetry config
publishUrl := strings.TrimSuffix(repoUrl.String(), "simple")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to have caused a regression for using jf poetry install with configured sources now unable to reach /simple index

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Automatically generated release notes safe to test Approve running integration tests on a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants