Add --non-interactive flag

[sco1]

Per #334, this adds a --non-interactive flag that aborts (via a new NonInteractive exception) instead of interactively prompting for username and/or password if the required credentials are missing.

Fixes #334.

[sco1]

A few questions on the testing:

Going through the existing tests, I believe there only needs to be tests added to test_utils:

• test_no_username_non_interactive_aborts
• test_no_password_non_interactive_aborts
• test_get_username_keyring_missing_non_interactive_aborts
• test_get_password_keyring_missing_non_interactive_aborts

Am I missing anything that should be covered?

Also, as a result of the changes to the signature of utils.get_username and utils.get_password, some of the existing tests will error out. Should I add a default to the utils or would it be a better idea to adjust the existing tests?

[zooba]

The logic and CLI looks good to me, though of course the maintainers will need to review.

Thanks for jumping on this, @sco1!

[jaraco]

Thank you for working on this issue. It seems to be quite important.

A few concerns:

• The implementation as proposed weaves a boolean variable through several function calls. I'll want to first identify a refactoring that limits the number of code blocks that this one option touches.
• I'd prefer instead of a boolean option to provide more descriptive options like "prompt: disable" or "secrets-from: env,keyring,pypirc,prompt" (a broader technique that lets the caller define or exclude the techniques attempted; there's already an option to bypass keyring).
• Does the implementation support TWINE_NON_INTERACTIVE environment variable in addition to the command line option? I think it should.
• (more of a comment than an action) I don't think this implementation prevents keyring from raising an interactive prompt. I think that's reasonable, given that the user could avoid keyring implementations that might be interactive or disable keyring entirely if appropriate. Unless keyring provided a similar non-interactive awareness, there's not much twine could do to relay that information.

[zooba]

• I don't think this implementation prevents keyring from raising an interactive prompt. I think that's reasonable, given that the user could avoid keyring implementations that might be interactive or disable keyring entirely if appropriate. Unless keyring provided a similar non-interactive awareness, there's not much twine could do to relay that information.

Oh that's a good point. Perhaps, since you're here, we could just decide to support KEYRING_NON_INTERACTIVE and add it to the keyring docs (as it's only really a suggestion to extenders), and then Twine can set the variable before invoking it?

• I'd prefer instead of a boolean option to provide more descriptive options like "prompt: disable" or "secrets-from: env,keyring,pypirc,prompt" (a broader technique that lets the caller define or exclude the techniques attempted; there's already an option to bypass keyring).

The downside of the secrets-from approach is that now the caller needs to remember to specify every other source, when really they're all meant to be ambient sources of information. I expect you'll very quickly see people saying "specify TWINE_USERNAME and --secrets-from env to make it work properly".

We have a long enough history of simple do-not-prompt options that I think it's fine to keep it simple. (The "bypass" keyring option is specifically to bypass the import keyring to make sure that a broken keyring package doesn't spoil Twine completely - it shouldn't need to be a precedent for making every method optional, only those that have external dependencies.)

[bhrutledge]

@jaraco looks like there are some follow-up questions to your suggestion. Can you offer feedback?

Also, I second this suggestion:

The implementation as proposed weaves a boolean variable through several function calls. I'll want to first identify a refactoring that limits the number of code blocks that this one option touches.

[jaraco]

I agree with Zooba. Simple is better. Let's just see if we can get the code to be less intrusive and iterate from there... or maybe accept the change and defer refactoring for another day.

Perhaps, since you're here, we could just decide to support KEYRING_NON_INTERACTIVE...

I was actually traveling and just eeked out my suggestions between flights.

I'm not sure a KEYRING_NON_INTERACTIVE option is viable. Keyring itself often doesn't have control over whether a particular call will be interactive. Some calls are interactive only on the first call but not subsequent ones. Keyring itself doesn't do any prompting.

[sco1]

I should have asked this a while ago, but just to make sure I haven't assumed incorrectly:

The implementation as proposed weaves a boolean variable through several function calls. I'll want to first identify a refactoring that limits the number of code blocks that this one option touches.

Let's just see if we can get the code to be less intrusive and iterate from there..

Am I correct in assuming this is intended as a PyPA task? With the help of the review comments I've tried to make the value as least touchy as possible but I'm not sure how much further I know how to take it without refactoring the configuration system itself, which I'm not familiar enough with Twine to feel comfortable undertaking.

I believe adding the env var support is a one-liner (+tests), but wanted to make sure the config system wasn't being refactored before pushing.

[jaraco]

I believe adding the env var support is a one-liner (+tests), but wanted to make sure the config system wasn't being refactored before pushing.

Today, I took a look at implementing the refactoring, and I wasn't happy with anything that I could come up with on short notice, but I have some more ideas I'll explore later. And so no, I don't want to block this effort any more. I'll accept the change as proposed, and you can add support for the environment variable in a separate PR.

[sco1]

Will take a look at the env var this week, thanks for all the help & feedback!