-
Notifications
You must be signed in to change notification settings - Fork 5
feat: add auth functions and add arg to covidcast #79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Our API supports auth via three approaches:
I won't implement option 1, so we don't pollute the user's request URLs and reduce chance of them sharing their API key. The latter two seem equivalent, so I'll implement the (2), since that's implicit in |
* remove set_auth and update README recipes instead * change API key prio to env var first, then option * use httr::authenticate to set query auth key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice! Have a bunch of little questions/suggestions. Main things:
- add mention of query complexity limit
- typo & tweaks in warning message
- testing and CI stuff
question: Are the old-epidata-endpoint auth token arguments clearly different from the new api key? We might need to go through and say something like "an {endpoint name}-specific authentication token (not the same thing as the newer, system-wide API key)". |
Yes, they are different. Agreed. |
An implementation of API keys: