fix: make login() username and password optional - #257
Shubham-Padkonde wants to merge 1 commit into
Conversation
Registry.login() prompts for the username and password when they are not given, but both were required positional parameters, so calling client.login() raised "TypeError: missing 2 required positional arguments" and the prompts were unreachable without passing None explicitly. Closes oras-project#235 Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>
vsoch
left a comment
vsoch
left a comment
There was a problem hiding this comment.
Let's work on this one next.
This is not in the lines that you changed, but I realize that this will show the password in the terminal:
password = input("Password: ")Could we please use getpass.getpass("Password: ").
Can you please test password_stdin=True without a username? E.g.,
client.login(password_stdin=True, hostname="x")Can you test providing the password but not the username? I want to make sure it does not return login not successful, which is misleading.
Finally, let's put all these changes under this same version update, so we can do one at a time and rebase accordingly.
| :type username: str | ||
| :param password: the user account password | ||
| :param password: the user account password, prompted for if not provided | ||
| :type password: str |
There was a problem hiding this comment.
It looks like the types here need to be updated. I also see that there is "insecure" which is not supported here anymore I don't think?
| answers = iter(["alice", "secret"]) | ||
| monkeypatch.setattr("builtins.input", lambda prompt="": next(answers)) | ||
| set_basic_auth = Mock() | ||
| monkeypatch.setattr(client.auth, "set_basic_auth", set_basic_auth) |
There was a problem hiding this comment.
Your docker client stub ignores its arguments, so the test won't catch the prompted credentials not reaching client.login(...).
|
@Shubham-Padkonde you've opened a lot of PRs, and we need to do one by one. This is the one we can work on next. Thanks! |
This branch has not been deployed
2 participants
Registry.login() prompts for the username and password when they are not given, but both were required positional parameters, so calling client.login() raised "TypeError: missing 2 required positional arguments" and the prompts were unreachable without passing None explicitly.
Closes #235