Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publish: Password requires username #8045

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Conversation

konstin
Copy link
Member

@konstin konstin commented Oct 9, 2024

You can't use a password without a username in uv publish, which we can catch in clap directly.

New error when using only a password:

$ uv publish --password dummy
error: the following required arguments were not provided:
  --username <USERNAME>

Usage: uv.exe publish --username <USERNAME> --password <PASSWORD> [FILES]...

For more information, try '--help'.

Fixes #8023

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

This looks like a good change, but I'm still wondering if we can guide people to success here. Can we add a custom hint about tokens?

@konstin
Copy link
Member Author

konstin commented Oct 9, 2024

What about adding it in the docstring of --password?

@zanieb
Copy link
Member

zanieb commented Oct 9, 2024

It would probably be best to include the hint in the error message, which would mean checking later manually instead of using Clap.

The short help won't show it if it's in the docstring.

@charliermarsh
Copy link
Member

Yeah can we just add a hint when we see Failed to publish... with a password but no username? You can even use Miette if you want to make it a bit easier to render the hint. See crates/uv/src/commands/diagnostics.rs for an example.

@zanieb
Copy link
Member

zanieb commented Oct 9, 2024

I think actually some indexes don't even require a username, just a password — this was an annoyance with twine in packse where I was forced to provide a dummy username — so we probably shouldn't enforce this until there's a failure?

@konstin
Copy link
Member Author

konstin commented Oct 10, 2024

Do you have an example for such an index?

@charliermarsh
Copy link
Member

@konsti -- Is there any objection to adding a real hint to the error trace? Can we just do that?

@konstin konstin force-pushed the konsti/password-requires-username branch from 32806b0 to 1357ca0 Compare October 15, 2024 07:44
@konstin konstin enabled auto-merge (squash) October 15, 2024 07:45
@konstin konstin merged commit dda91d4 into main Oct 15, 2024
61 checks passed
@konstin konstin deleted the konsti/password-requires-username branch October 15, 2024 08:01
@charliermarsh
Copy link
Member

@konstin -- What about Zanie's concern above? I thought the conversation was veering towards only showing this when we hit a failure.

@@ -75,6 +75,10 @@ pub(crate) async fn publish(
(username, password)
};

if password.is_some() && username.is_none() {
bail!("You need to provide a username with a password, use `--token` for tokens");
Copy link
Member

Choose a reason for hiding this comment

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

I think we should suggest --user and --token, and note the env vars since it's common in GHA:

Attempted to publish with a password, but no username. Either provide a username with --user (UV_PUBLISH_USERNAME), or use --token (UV_PUBLISH_TOKEN) in lieu of a password.

Copy link
Member Author

Choose a reason for hiding this comment

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

This case only appears when you use only a password and not a username, despite token vs username/password are clearly documented in the pypi docs and our docs, and other indexes that i have tested providing clear guidance on username and password values. This is a fringe case and I'd rather spend our efforts on more impactful UX aspects, such as showing good messages after 403 errors, guiding users to the correct github actions trusted publishing settings, skip-existing/retries, etc.

@konstin
Copy link
Member Author

konstin commented Oct 15, 2024

I thought the concern was that we should tell users about tokens?

We definitely can't support password-only without an example of an index and how that index would encode this in the HTTP message, the current serialization always requires a username.

@charliermarsh
Copy link
Member

Maybe @zanieb can provide an example then. If it's true then this could break users as-is.

@konstin
Copy link
Member Author

konstin commented Oct 15, 2024

There may be other use cases we need to support, but it is not a regression due to:

if let (Some(username), Some(password)) = (username, password) {

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 16, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.21` -> `0.4.22` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.4.22`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0422)

[Compare Source](astral-sh/uv@0.4.21...0.4.22)

##### Enhancements

-   Respect `[tool.uv.sources]` in build requirements ([#&#8203;7172](astral-sh/uv#7172))

##### Preview features

-   Add a dedicated `uv publish` error message for missing usernames ([#&#8203;8045](astral-sh/uv#8045))
-   Support interactive input in `uv publish` ([#&#8203;8158](astral-sh/uv#8158))
-   Use raw filenames in `uv publish` ([#&#8203;8204](astral-sh/uv#8204))

##### Performance

-   Reuse the result of `which git` ([#&#8203;8224](astral-sh/uv#8224))

##### Bug fixes

-   Avoid environment check optimization for `uv pip install --exact` ([#&#8203;8219](astral-sh/uv#8219))
-   Do not use free-threaded interpreters without a free-threaded request ([#&#8203;8191](astral-sh/uv#8191))
-   Don't recommend `--prerelease=allow` during build requirement resolution errors ([#&#8203;8192](astral-sh/uv#8192))
-   Prefer optimized builds for free-threaded Python downloads ([#&#8203;8196](astral-sh/uv#8196))
-   Retain old `python-build-standalone` releases ([#&#8203;8216](astral-sh/uv#8216))
-   Run `uv build` builds in the source distribution bucket ([#&#8203;8220](astral-sh/uv#8220))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uv publish giving Missing credentials for https://upload.pypi.org/legacy/
3 participants