Conn auth

[gidden]

Please confirm that this PR has done the following:

• Tests Added
• Documentation Added
• Description in RELEASE_NOTES.md Added

This PR adds the ability to connect to non-public databases by providing a username and password.

TODO:

• this currently also requires knowledge of an "application" name. For example, for the SR1.5 explorer, the "short name" is iamc15 and the "application name" is IXSE_SR15. We should decide which to go with in the end. To note, utilizing the "short name" will require an additional mapping in the python code (to the best of my knowledge), which was seen as desirable to avoid. In any case, I think we should limit the user to needing to know only one name.
• decide on how to pass over credentials. Options: 1) leave as is and it is up to the user to store them properly. 2) support reading/writing from a config file. 3) support passing over env variables. 4) both 2 & 3
• add functionality to check success of requests (likely as a context manager)

[coveralls]

Coverage increased (+0.2%) to 91.083% when pulling a473118 on conn-auth into a134b46 on master.

[gidden]

@byersiiasa if you want to get started working on AR6, you can use this branch to do so. You'll need to coordinate the naming conventions with @zikolach and @fonfon

[fonfon]

For the application names I'd prefer to fetch them from the auth-server and see how that goes. If we hard-code a lookup dict here it will become tedious (or forgotten) to keep it up to date, even if the application names do not change very often.
Users would then have to provide the application name though, like listed on https://data.ene.iiasa.ac.at/auth/#/apps

[gidden]

Ok @fonfon, happy to make this more amenable to the existing REST API features. Let's discuss when we're both in the office/on slack.

[gidden]

Here is the example how users can currently interact with the pyam.iiasa.Connection class: https://pyam-iamc.readthedocs.io/en/stable/tutorials/iiasa_dbs.html

[byersiiasa]

@byers if you want to get started working on AR6, you can use this branch to do so. You'll need to coordinate the naming conventions with @zikolach and @fonfon

thanks I'll try this today - for some reason I didn't get a notification

[gidden]

Note that this needs iiasa/auth_server#53 to continue further

[gidden]

Hi folks, from my perspective this PR is now complete and ready for final review. @fonfon @byersiiasa and/or @danielhuppmann would you mind taking a final look?

[danielhuppmann]

One minor comment on a docstring, otherwise looks good.

But I see the risk with that approach that users will store their credentials in notebooks, maybe even committing those to GitHub by mistake. Any thoughts on that?

More generally, I think it is not ideal that the API uses database-instance-names and not "nice" names like "iamc15", @fonfon @zikolach. Maybe that was the easier implementation for a quick solution, but sub-optimal from a user-experience point-of-view.

[zikolach]

@gidden please have a look on comments

[gidden]

@danielhuppmann @zikolach @fonfon (and @khaeru if you're interested)

I have listed a set of TODOs in the PR summary. Could you please provide input/opinions on items 1 and 2?

Namely, for credentials, I see a lot of potential problems here. E.g., if we want to support environment variables explicitly, once they are set, a user can no longer get explicitly to public dbs as "anonymous", because any sort of default/fall-back will always see them. Config files are less problematic, but is it our job in this library to support this? A user can always write their own file and read it in however they wish...

On the naming, I have no super strong opinions and would be happy if y'all could come to a consensus that I can implement.

[gidden]

Hi all, in bea472e I have added the ability to pass a credentials file with a warning if a non-file based option is used. I think that should cover the issues here.

[gidden]

Ok all, to my eye this PR is now complete. Please let me know the resolution of point 1 between yourselves.

[byersiiasa]

Hi
I have just tested (limited) and seems to work well. In particular - tested it that when credentials are supplied, then more instances are available... nice!

# Connection with instance / creds
Connection(creds=('xxxx','XXXXX')).valid_connections

returns
['IXSE_AR6', 'IXSE_SETNAV', 'IXSE_SR15', 'IXSE_SR15_QA', 'IXSE_TEST_PUBLIC']

# Connection without instance / creds
 Connection()

returns
['IXSE_SETNAV', 'IXSE_SR15', 'IXSE_SR15_QA', 'IXSE_TEST_PUBLIC']

Thanks all!

[gidden]

Hi @danielhuppmann @zikolach and @fonfon would be great to try to come to some resolution on the point above so this can be merged. Thanks!

[gidden]

Hi @danielhuppmann @zikolach and @fonfon. This has been an open for ~2 weeks now. It would be great to get a response on point 1 above. If there is no resolution in a reasonable timeframe, I suggest we pull this as-is unless someone wants to object and review!

[zikolach]

@gidden fyi I have no access to mark this PR as accepted despite the fact I included in reviewers list

[gidden]

Just added you as a collaborator!

[zikolach]

Just added you as a collaborator!

@gidden half way there - now I can squash and merge but not accept/reject ;)

[zikolach]

Thanks @gidden

[byers]

Hello, I think someone included Github id reference @byers instead of @byersiiasa in a pull request comment. This might explain why he's not getting notifications of comments, etc. since I am ;-). It looks like an interesting project, but i'm pretty busy with a startup and other projects, so please remove me from your list (@byersiiasa instead of @byers) https://github.com/byersiiasa Thanks for your time, Byers

…

On Mon, Jul 29, 2019 at 2:43 AM Nikolay Kushin ***@***.***> wrote: ***@***.**** approved this pull request. Thanks @gidden <https://github.com/gidden> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#243?email_source=notifications&email_token=AAHSE3PHCQ7ZH5J57K3NAATQB2URPA5CNFSM4H63OPC2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7Z6IEI#pullrequestreview-267641873>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHSE3LKDTEPCLUKMQXA5CLQB2URPANCNFSM4H63OPCQ> .

[danielhuppmann]

Thanks @gidden and @zikolach for the effort!

Let's use the "application names" and maybe add support for "short names" on the authentication API at a later stage.