Publish: Allow empty token, so twine uses keyring #1203
Conversation
| @@ -77,17 +77,16 @@ pub fn execute(cmd: Args) -> Result<(), Error> { | |||
| // c. Otherwise prompt for token and provide encryption option, storing the result in credentials. | |||
| let repository = &cmd.repository; | |||
| let mut credentials = get_credentials()?; | |||
| credentials | |||
| let repo_credentials = credentials | |||
There was a problem hiding this comment.
This is a bit tangential, but this lets us pass less info to secret_from_token, just the repo credentials rather than the whole thing.
Happy to revert if unwanted, but it does remove a bunch of extra gets to this value, and gives us a proof that the key exists, instead of having to handle the Option returned by .get(repository)
| .get(repository) | ||
| .and_then(|table| table.get("token")) | ||
| let maybe_secret = if let Some(token) = cmd.token { | ||
| secret_from_token(token, cmd.yes, repo_credentials)? |
There was a problem hiding this comment.
Made this helper to deduplicate the common logic between this cmdline case and the prompt case below
| repo_credentials: &mut Item, | ||
| ) -> Result<Option<Secret<String>>, Error> { | ||
| if token.is_empty() { | ||
| Ok(None) |
There was a problem hiding this comment.
this is the key bit. instead of bailing on the empty string, we return a None for the secret
| .arg("--repository-url") | ||
| .arg(repository_url.to_string()); | ||
| if let Some(secret) = maybe_secret { |
There was a problem hiding this comment.
the other key bit. we only add a password if a secret got created, ie if there was a non-empty string (or some credential looked up from file)
|
@charliermarsh @mitsuhiko would you be able to take a look at this when you get a chance? happy to amend the approach as needed |
Similar in spirit to #759, but less ambitious.
This just makes rye accept empty-string
tokenvalues. In such a case, rye just doesn't pass a--passwordvalue ontotwine. This is valid from twine's perspective; in this case twine will fall back to using its keyring support.