astropy
MNT: Remove explicit password arguments from methods
We recently noticed (e.g. see discussion here: https://github.com/astropy/astroquery/pull/2386#discussion_r863120399) that there is some usage of explicit password arguments in the user facing API for several modules. We should clean up all of these and instead use keyring, prompting for password, environ variables, etc. I don't have a concrete recipe we have to follow, please feel free to use this issue for discussions. The aim would be to do something very similar over all our modules. Once we reach an ideal solution, we should document it, too either in the template or narrative docs.
The testing part of this question is most certainly not solved as that issue has been raised in #2367 already.
I'm pinging the authors/maintainers of the relevant modules.
cc @esdc-esac-esa-int, @andamian, @keflavich, @olyoberdorf
Just looking at the alma approach, it's certainly easy to do. I don't really see the benefit other than consistency. To make sure I understand, password would no longer be allowed for the _login() implementations? So not just a case of making it optional and falling back to keychain?
What happens if the user is running in a hosted Jupyter notebook? Will they have a personal keychain in that Linux environment or might it end up in some server user's keychain for instance? What if someone wants to run a script non-interactively, like an overnight job? I suppose then we could add the environment variable option as another layer before prompting. But for the hosted Jupyter notebook, I worry we'd end up having them call os.setenv() and checking the module docs for what environment variable it is expecting.
Just looking at the alma approach, it's certainly easy to do. I don't really see the benefit other than consistency. To make sure I understand, password would no longer be allowed for the _login() implementations? So not just a case of making it optional and falling back to keychain?
Yes, I think the main point should be that no plain text password will end up in notebooks, python history, etc. So it will be always taken in from a keyring, a token, config files, or from the prompt.
It's a very good point you bring up with the hosted notebooks. I suppose if they are provided a personalized and persistent home directory we can assume that they can either have a keychain, or set the env variables or config files that are reusable.
After trying it out on Datalab's Jupyter service, keychain does not work there. Although they do grant users shell accounts as well, environment variables set in the .bashrc do not show up in Jupyter sessions either. Setting an env var directly in the Jupyter session does work - not a recommendation, just an observation.
OK, so that means that password storage is not working out of the box, but I assume it still prompts for a password and you need to do that only once in a notebook, right? (I'm afraid I have to set up personal accounts before I can reasonably test it myself.)
(This is what I see:
)
Yes, sorry, that's right. So it's covered that way. I tried alma on Datalab just now and got the same.
The existing argument to the _login doesn't force the user to have the password in clear. They have the freedom (and responsibility) to access it with whatever technology they have available, including keyring. I'm wondering if, by offering this convenience, we don't inadvertently limit their options.
Hi all, this is the same for the TAPPlus used on ESA modules, the user can simply call login() and then the user and password will be requested in the console.
