Code review comment for lp:~leonardr/launchpadlib/separate-login-for-cronjob

Revision history for this message
Leonard Richardson (leonardr) wrote :

> So, these names then are essentially aliases to (pseudocode)
> login(... store=keyringstore)
> login(... store=filestore)
>
> why not expose that instead.
> login(... store=None)
> """
> store: a store to request credentials from. If None, GnomeKeyringStore
> will be used, as the most reliable and secure store we have available
> today. Using a different store (see <...> for a list of stores) is
> needed when running scripts outside of a desktop context such as from
> cron.
> """

I'm fine with this. The main problem is that we have already staked out the namespace of generic names. login() and login_with() are existing methods that I hope to deprecate with this branch.

I think it wouldn't be _too_ bad to change the behavior of login() in 1.9.0, since everyone I know of uses login_with(). We could even call it 2.0.0. (But, I think a 3.0.0 would not be far behind--I was planning to make another big backward-incompatible change, though I don't remember what it was at the moment.)

Without an alias for cron usage, the code to run a cron script would look like this:

store = UnencryptedFileCredentialStore(credentials_file)
launchpad = Launchpad.login("my application" "production", credential_store=store)

Martin, are you OK with this?

« Back to merge proposal