-
Notifications
You must be signed in to change notification settings - Fork 180
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
docs: rework API keys documentation #363
base: main
Are you sure you want to change the base?
Conversation
9640be6
to
7f5beb0
Compare
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.
Love the direction of this change but want to change the wording a bit.
docs/handling-api-keys.md
Outdated
All pgai functions that require an API key take two optional parameters: | ||
`api_key_name`, and `api_key`. If neither parameter is provided, the function | ||
will fall back to a default name and attempt to extract an API key with that | ||
name. TODO: is this clear? | ||
|
||
We suggest that you generally prefer using `api_key_name`, as it is harder to | ||
misuse. |
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.
All pgai functions that require an API key take two optional parameters: | |
`api_key_name`, and `api_key`. If neither parameter is provided, the function | |
will fall back to a default name and attempt to extract an API key with that | |
name. TODO: is this clear? | |
We suggest that you generally prefer using `api_key_name`, as it is harder to | |
misuse. | |
Since API keys are sensitive values, we provide several ways to specify them so they can be | |
provided securely: | |
**Recommended ways (most secure) ** | |
- If you are using Timescale Cloud, we recommend (setting the API key)[#TODO-link-section-below] through the console and then specify the env variable name via the `api_key_name` function parameters. | |
- If you are self-hosting, you can (provide them as environmental variables to the database)[#TODO-link-section-below] and then specify the env variable name via the `api_key_name` function parameters. | |
** Other ways ** | |
1. You can provide them as environmental variable to interactive psql session and then specify the env variable name via the `api_key_name` function parameters. | |
1. You can provide the value directly via the `api_key` function parameters. | |
By default, the system to extract an API key with the default `api_key_name`. |
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.
** Other ways **
1. You can provide them as environmental variable to interactive psql session and then specify the env variable name via the `api_key_name` function parameters.
I would say "You can provide them as a session-level GUC to interactive psql session..."
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.
b/c unless I'm mistaken, an environment variable on the psql client side won't work
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.
I've applied the suggestions that you proposed. PTAL.
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.
lgtm
docs/handling-api-keys.md
Outdated
We suggest that you generally prefer using `api_key_name`, as it is harder to | ||
misuse. | ||
|
||
### Using the `api_key` parameter |
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.
### Using the `api_key` parameter | |
### Provide the API key value directly using the `api_key` parameter |
docs/handling-api-keys.md
Outdated
\g | ||
``` | ||
|
||
### Using the `api_key_name` parameter |
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.
I would break out the subsections here into their own sections and move the two recommended ones on top
docs/anthropic.md
Outdated
@@ -9,21 +9,11 @@ This page shows you how to: | |||
|
|||
Anthropic functions in pgai require an [Anthropic API key](https://docs.anthropic.com/en/docs/quickstart#set-your-api-key). | |||
|
|||
- [Handle API keys using pgai from psql](#handle-api-keys-using-pgai-from-psql) | |||
- [Handle API keys using pgai from python](#handle-api-keys-using-pgai-from-python) | |||
The simplest option for interactive use is to configure the API key as a |
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.
not sure about this text since the session level parameter isn't the recommended method. Let's think.
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.
What do you propose?
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.
The simplest option for interactive use is to configure the API key as a | |
In production, we suggest setting the API key using an environment variable. During testing and development, it may be easiest to pass the key value in directly using the `api_key` parameter. For more options and details, consult the | |
[Handling API keys](./handling-api-keys.md) document. |
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.
I'm not really happy with this suggestion. I agree with telling users to use an env var as first prio. I don't think that we should suggest passing the key value directly (instead of the session-level parameter). The main reason I don't like it is that it will definitely result in the secret being exposed in the logs.
docs/handling-api-keys.md
Outdated
There are four ways to configure an API key: | ||
|
||
- In the Timescale Cloud Project settings | ||
- In an interactive psql session |
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.
There are two flavors of interactive psql session:
- session level guc
- psql variable with \bind
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.
the session level guc option is probably "better" but I'd like to keep the instructions about using a psql variable too
Why? |
Why not? It's a valid option that might appeal to some folks. Also, I'd personally like to have the instructions around. Also, it demonstrates an approach that is similar to how you might do it from a programming language (i.e. a parameterized query). |
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.
my suggestion for the entry text. Should be in multiple places
docs/anthropic.md
Outdated
@@ -9,21 +9,11 @@ This page shows you how to: | |||
|
|||
Anthropic functions in pgai require an [Anthropic API key](https://docs.anthropic.com/en/docs/quickstart#set-your-api-key). | |||
|
|||
- [Handle API keys using pgai from psql](#handle-api-keys-using-pgai-from-psql) | |||
- [Handle API keys using pgai from python](#handle-api-keys-using-pgai-from-python) | |||
The simplest option for interactive use is to configure the API key as a |
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.
The simplest option for interactive use is to configure the API key as a | |
In production, we suggest setting the API key using an environment variable. During testing and development, it may be easiest to pass the key value in directly using the `api_key` parameter. For more options and details, consult the | |
[Handling API keys](./handling-api-keys.md) document. |
@JamesGuthrie @jgpruitt I'd like to reduce the number of options, but that can wait for another PR |
a2b2973
to
aee5f2f
Compare
aee5f2f
to
f006c66
Compare
Note: this is a somewhat opinionated refactoring of our API key documentation.
The main problem I'm trying to solve is the same repeated (but slightly different) instructions for every provider.
To this end, I extracted out the common bits, and left enough to get started in for each provider.
In extracting out the common bits, I have removed some of the instructions that we previously had. In particular I have cut out the "Handle API keys using pgai from python" section. The following paragraph is now completely removed: