Skip to content
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

Doc clarification: sqlserver.connect secret_id #871

Closed
mdavis-xyz opened this issue Aug 20, 2021 · 3 comments
Closed

Doc clarification: sqlserver.connect secret_id #871

mdavis-xyz opened this issue Aug 20, 2021 · 3 comments
Labels
question Further information is requested

Comments

@mdavis-xyz
Copy link
Contributor

mdavis-xyz commented Aug 20, 2021

The documentation for sqlserver.connect says:

secret_id (Optional[str]:) – Specifies the secret containing the version that you want to retrieve. You can specify either the Amazon Resource Name (ARN) or the friendly name of the secret.

I have 2 questions.

  1. Why does it say "containing the version that you want to retrieve". Is that a typo? Version of what? Secrets don't have version. The version of SQL Server I'm using or ODBC driver version is not a secret, that's just config.

  2. What should the secret be structured like?
    e.g. is the entire secret value considered the password? (If so, where do I put the username?)
    Does it expect:

{
   "user": "john",
   "password": "blah"
}

or

{
   "username": "john",
   "password": "blah"
}

or something else?

I note that there's no mention of secret on the SQL tutorial page.

Looking here, it seems it wants:

{
    "username": ...
    "password": ...
    "host": ...
    "port": ...
}

and optionally dbname.

Is that right?
If so, let's update the documentation for .connect() to say those fields are expected in the secret.

@mdavis-xyz mdavis-xyz added the question Further information is requested label Aug 20, 2021
@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Aug 20, 2021

It seems engine: "sqlserver" is required too.

@mdavis-xyz
Copy link
Contributor Author

A related doc issue, the example for sqlserver.read_sql_query is not formatted correctly.
We need to add a blank line here.

@jaidisido
Copy link
Contributor

Thanks for raising this @mdavis-xyz I have clarified the docs accordingly in this commit. It's visible in the latest docs.

Also fixed the minor issue with read_sql_query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants