Skip to content

Store auth token in env#46

Open
jashapiro wants to merge 8 commits into
jashapiro/update-datasetsfrom
jashapiro/env_token
Open

Store auth token in env#46
jashapiro wants to merge 8 commits into
jashapiro/update-datasetsfrom
jashapiro/env_token

Conversation

@jashapiro
Copy link
Copy Markdown
Member

Closes #44
STacked on #45

I decided to wait on final downloads until I had updated to use an environment-stored token. This makes it easier to make calls with all the same token instead of having to pass it in on every call. It is also somewhat easier for users to persist tokens between sessions if desired.

Because the auth_token parameter is no longer usually required, I moved it to the end of the external calls. This does break download_project() if people were using positional calls, but I think that is worth the error. I added a warning to hopefully give people a reason to understand why their download folder is a uuid, but in most cases I would expect the calls to fail anyway, so the warning is just an extra. (I didn't make it an error because we do name datasets with uuids too, and I can imagine people actually using those as download directories, however rarely).

@sjspielman
Copy link
Copy Markdown
Member

Because the auth_token parameter is no longer usually required, I moved it to the end of the external calls.

Before I review, this was a surprising sentence to read - are we no longer authenticating...?

@jashapiro
Copy link
Copy Markdown
Member Author

Because the auth_token parameter is no longer usually required, I moved it to the end of the external calls.

Before I review, this was a surprising sentence to read - are we no longer authenticating...?

No, we are, just in the normal course of events it will be read from an environment variable automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants