Support custom authentication handlers for private pypi. #8250
Conversation
fedorbirjukov
force-pushed
the
auth-plugins
branch
from
8e64bb0
to
80a251c
Compare
fedorbirjukov
force-pushed
the
auth-plugins
branch
from
80a251c
to
373ce5e
Compare
| auth_class = kwargs.pop("auth_class", None) | ||
| if auth_class is None: | ||
| auth_class = MultiDomainBasicAuth |
There was a problem hiding this comment.
| auth_class = kwargs.pop("auth_class", None) | |
| if auth_class is None: | |
| auth_class = MultiDomainBasicAuth | |
| auth_class = kwargs.pop("auth_class", MultiDomainBasicAuth) |
There was a problem hiding this comment.
That does not work because this named argument is always provided.
There was a problem hiding this comment.
Thanks, I missed that. How about provide it with MultiDomainBasicAuth as suggested below?
There was a problem hiding this comment.
I tried that at first. But None is a better default IMO. This way cli is not coupled with network. Currently MultiDomainBasicAuth is not a plugin, it's the default implementation.
|
|
||
| auth_plugin = partial( | ||
| PipOption, | ||
| '--auth', '--auth-plugin', |
There was a problem hiding this comment.
As an user, I would find this to be confusing.
There was a problem hiding this comment.
Not sure what's confusing about this. It's quite common for options to have multiple names.
There was a problem hiding this comment.
I find auth and auth-plugin to have different first-impression meaning, but I don't represent all users so no worries, stick to what you think is best.
There was a problem hiding this comment.
Thanks for clarifying. It's similar to the '--src' option, which has three longer names: '--source', '--source-dir', '--source-directory'. The longer the name - the more informative and clear it is. Shorter names are easier to type. Let's see if others find it confusing, too.
@McSinyx Don't worry. Your thoughts and perceptions are welcome. Thanks for thinking with me.
| dest='auth_class', | ||
| type='choice', | ||
| metavar='plugin', | ||
| default=None, |
There was a problem hiding this comment.
I think we might want to have MultiDomainBasicAuth here.
There was a problem hiding this comment.
I considered this. But didn't want to couple cli and network code.
| help='Specify authentication plugin to be used: {}.'.format( | ||
| ', '.join(discovered_auth_plugins.keys()) or 'no plugins found' |
There was a problem hiding this comment.
I wonder if we can phrase this better in case no plugin found. Personally I'm bad at making concise sentences so I can't offer any suggestion though.
|
Hello! I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the |
|
Closing due to lack of movement here. Please feel free to file a new PR if you want to get the ball rolling again! :) |
PR for #4475
MultiDomainBasicAuth is no longer the only option, it's just the default implementation.
Use
--authoption to specify the plugin/handler that should be used.Help message for this option lists all available/installed authentication plugins.
A plugin consists of:
class Auth, and"pip.auth_plugins": "win-sso=pip-win-sso-auth"