744 default api key #752
744 default api key #752
Conversation
cartoframes/auth/__init__.py
Outdated
| @@ -20,6 +20,14 @@ def set_default_context(context=None, base_url=None, api_key=None, creds=None, s | |||
| api_key='your api key' | |||
| ) | |||
|
|
|||
| From a base_url (for public datasets) | |||
There was a problem hiding this comment.
I like the idea of educating the user about what's happening. How about saying something about how default_public is used for the API key in this case?
cartoframes/auth/__init__.py
Outdated
| @@ -6,7 +6,7 @@ | |||
| _default_context = None | |||
|
|
|||
|
|
|||
| def set_default_context(context=None, base_url=None, api_key=None, creds=None, session=None): | |||
| def set_default_context(context=None, base_url=None, api_key='default_public', creds=None, session=None): | |||
There was a problem hiding this comment.
I'm missing the Args section of the docs here. Is that going to happen package-wide in another PR?
There was a problem hiding this comment.
Yes, I'll add it in this PR
cartoframes/auth/__init__.py
Outdated
| @@ -6,7 +6,7 @@ | |||
| _default_context = None | |||
|
|
|||
|
|
|||
| def set_default_context(context=None, base_url=None, api_key=None, creds=None, session=None): | |||
| def set_default_context(context=None, base_url=None, api_key='default_public', creds=None, session=None): | |||
There was a problem hiding this comment.
It feels a little strange to me that context can be used for a base_url or a cartoframes.Context object, but it also makes some logical sense that a base url is essentially a context if there's a default api key.
Some ideas to get around this:
- force keyword args using the
*notation in the function signature, and let the first arg be a more generic thing - rename the
contextkwarg? - do nothing because it's fine :) But clear docs explaining what's going on
There was a problem hiding this comment.
Yes. The first param can be a context or a base_url. Renaming it to be more generic is a good approach to me.
Fixes: #744