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

Client connection reference page #12538

Merged
merged 13 commits into from Jan 12, 2022
Merged

Client connection reference page #12538

merged 13 commits into from Jan 12, 2022

Conversation

ericharmeling
Copy link
Contributor

@ericharmeling ericharmeling commented Dec 9, 2021

Fixes #11022.
Fixes https://cockroachlabs.atlassian.net/browse/DOC-1856.
Related to #12353.

This PR adds a client connection reference page with connection configuration information for all of our fully-supported client drivers and ORM frameworks.

I collected the information presented in this reference page from various internal documents (Google Docs, Jira tickets, other GH issues) and from third-party reference documentation. As of 1/6/22, all of these connection configurations have been tested, with the latest version of each client driver/ORM.

Note that the connection reference provided here is specific to cloud cluster connections.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@netlify
Copy link

netlify bot commented Dec 9, 2021

✔️ Netlify Preview

🔨 Explore the source changes: 1aac90e

🔍 Inspect the deploy log: https://app.netlify.com/sites/cockroachdb-docs/deploys/61ded91f0147f10008dddbba

😎 Browse the preview: https://deploy-preview-12538--cockroachdb-docs.netlify.app

Copy link
Contributor

@kernfeld-cockroach kernfeld-cockroach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, I think we should suggest a single preferred connection methodology, ideally URL, for every tool. I'd like to reduce the number of different paths that we need to maintain.

{{site.data.alerts.callout_danger}}
To connect to a {{ site.data.products.serverless }} cluster from a Python application, you must have a valid CA certificate located at `~/.postgresql/root.crt`.

For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see [Connect to a {{ site.data.products.serverless }} Cluster](../cockroachcloud/connect-to-a-serverless-cluster.html).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to not be rendering as a link on the Netlify preview although I don't entirely understand why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Jekyll has a hard time parsing markdown in these callouts...

<button class="filter-button page-level" data-scope="django">Django</button>
</div>

{{site.data.alerts.callout_danger}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this rises to the level of "danger/red;" I would reserve that for scary things like potential security vulnerabilities or things that could cause you to lose data. I would personally highlight this at like an "info" level. We probably have some kind of SOP for this but I'm not sure who is in charge of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

v21.2/connection-reference.md Outdated Show resolved Hide resolved
conn = psycopg.connect('{connection-string}')
~~~

Psycopg accepts the following format for CockroachDB connection strings:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary for us to document this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you are referring to with this comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was quite unsuccessfully attempting to articulate here is that, in an ideal world, we wouldn't even need to document the format of CockroachDB connection strings for Serverless because the user would simply copy the appropriate connection string directly from the CockroachDB Cloud web UI. However, I am now realizing that this is not immediately possible because the web UI doesn't generate the correct connection strings. So, I guess the first step would be for the web UI to generate the correct connection string for a particular tool. Once that's ready to go, then we could un-document this.

postgresql://{username}:{password}@{host}:{port}/{cluster_name}.{database}?sslmode=verify-full
~~~

You can also specify connection parameters to connect to CockroachDB with Psycopg:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that, for each tool, we pick a single preferred methodology (i.e. URL vs. parameters) and go with that. Is it necessary to document both? Between the two I would say that that URL is likely superior because it's only a single thing for the user to copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to provide users with as much information as could be useful, since this is technically a piece of reference documentation. I figure that for any given tool, some people might want to see the property-based connection configuration, and others might want the URL configuration.

But TBH, IDK if people really care all that much though about the configuration methodology. So I'll pick one for each tool.

Re: the URL being superior, I feel like that really depends on the tool. Some tools don't accept URLs. And for some tools (like Django), I think it's more common to use properties. In the case of Django, using a URL might actually be more complicated (it requires you to initialize an empty DATABASE property, import and use a third-party library, and you still need to specify the ENGINE property individually).

I did my best to make an educated guess on which is more common/easiest for each tool, but it would be nice to get a more nuanced assessment of each (perhaps @rafiss @jordanlewis might have opinions on the Java/Ruby ones in particular). Or we can just run with the educated guess and wait for user feedback/requests.

)
~~~

For more information about connecting with Psycopg, see the [official Psycopg documentation](https://www.psycopg.org/psycopg3/docs).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is linking to psycopg3, but we only support psycopg2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

(For some reason, I think we supported psycopg3 already...)

You can also specify the connection configuration as individual properties:

{% include copy-clipboard.html %}
~~~ js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to decide which of Client or Pool we recommend for most users, and only document that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (went with Pool)

v21.2/connection-reference.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR @kernfeld-cockroach !

See my responses inline.

<button class="filter-button page-level" data-scope="django">Django</button>
</div>

{{site.data.alerts.callout_danger}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

v21.2/connection-reference.md Outdated Show resolved Hide resolved
{{site.data.alerts.callout_danger}}
To connect to a {{ site.data.products.serverless }} cluster from a Python application, you must have a valid CA certificate located at `~/.postgresql/root.crt`.

For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see [Connect to a {{ site.data.products.serverless }} Cluster](../cockroachcloud/connect-to-a-serverless-cluster.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Jekyll has a hard time parsing markdown in these callouts...

postgresql://{username}:{password}@{host}:{port}/{cluster_name}.{database}?sslmode=verify-full
~~~

You can also specify connection parameters to connect to CockroachDB with Psycopg:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to provide users with as much information as could be useful, since this is technically a piece of reference documentation. I figure that for any given tool, some people might want to see the property-based connection configuration, and others might want the URL configuration.

But TBH, IDK if people really care all that much though about the configuration methodology. So I'll pick one for each tool.

Re: the URL being superior, I feel like that really depends on the tool. Some tools don't accept URLs. And for some tools (like Django), I think it's more common to use properties. In the case of Django, using a URL might actually be more complicated (it requires you to initialize an empty DATABASE property, import and use a third-party library, and you still need to specify the ENGINE property individually).

I did my best to make an educated guess on which is more common/easiest for each tool, but it would be nice to get a more nuanced assessment of each (perhaps @rafiss @jordanlewis might have opinions on the Java/Ruby ones in particular). Or we can just run with the educated guess and wait for user feedback/requests.

)
~~~

For more information about connecting with Psycopg, see the [official Psycopg documentation](https://www.psycopg.org/psycopg3/docs).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

(For some reason, I think we supported psycopg3 already...)

conn = psycopg.connect('{connection-string}')
~~~

Psycopg accepts the following format for CockroachDB connection strings:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you are referring to with this comment?

v21.2/connection-reference.md Outdated Show resolved Hide resolved
You can also specify the connection configuration as individual properties:

{% include copy-clipboard.html %}
~~~ js
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (went with Pool)

Copy link
Member

@bdarnell bdarnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 8 of 9 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connect-to-the-database.md, line 37 at r3 (raw file):

<div class="filter-content" markdown="1" data-scope="local">
Use the connection information provided by the [`cockroach` client](cockroach-commands.html) to configure your client to connect to CockroachDB.

This is referring to the output of cockroach start or start-single-node, right? In that case the cockroach command is acting as the server, not a client.

This page is pretty small now - do we still need it?


v21.2/connection-reference.md, line 54 at r3 (raw file):

{% include copy-clipboard.html %}
~~~
postgresql://<username>:<password>@<host>:<port>/<cluster_name>.<database>?sslmode=verify-full

Don't we want to prefer the cluster options parameter instead of putting the cluster name in the database name when it's supported?


v21.2/connection-reference.md, line 139 at r3 (raw file):

{{site.data.alerts.callout_info}}
To connect to a {{ site.data.products.serverless }} cluster from a Python application, you must have a valid CA certificate located at <code>~/.postgresql/root.crt</code>.<br>For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see <a href="https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html">Connect to a {{ site.data.products.serverless }} Cluster</a>.

Non-serverless clusters must also have a root certificate downloaded (in all languages, not just python and ruby).

We should probably have some explanation of what to do if you have to work with multiple clusters (or both cockroach and postgres). (You may be able to concatenate the files, or you may want to use multiple paths and specify them explicitly on the connection string depending on the security practices of the various CAs in question).


v21.2/connection-reference.md, line 414 at r3 (raw file):

{{site.data.alerts.callout_info}}
JDBC connection URLs do not accept the `username` and `password` parameters. These parameters must be set outside of the connection string.

I think JDBC does support username and password in the connection string, it's just in a different place than the others. Append &user={user}&password={password}. (I think I've tested this in the past, but not today so please confirm).

Copy link

@andy-kimball andy-kimball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 246 at r3 (raw file):

`{host}`  | The host on which the CockroachDB node is running.
`{port}`  | The port at which the CockroachDB node is listening.
`{cluster_name}`  | The name of the CockroachDB cluster.

This is misleading, because it's not the name of the cluster - it's the name of the cluster plus a unique number called the tenant_id. The problem is that two different customers could name their clusters the same thing, and we have to know which one to connect to. Therefore, we add a unique number that gives us that information.

I've recently written an in-depth description of all the problems with connection routing. I'd suggest reading through that in order to get all the context of why the problem is so difficult to solve in a simple way.

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR @bdarnell !

See my responses inline.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @ericharmeling, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connect-to-the-database.md, line 37 at r3 (raw file):

In that case the cockroach command is acting as the server, not a client.

Error on my part. Fixed.

This page is pretty small now - do we still need it?

I don't think so, actually... But removing it might be out of the scope of this PR, as this page is referenced throughout our docs and those references will require some redirection/rewriting. I've opened a docs issue to handle a rework of this whole "Connect" section in a separate PR.


v21.2/connection-reference.md, line 54 at r3 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Don't we want to prefer the cluster options parameter instead of putting the cluster name in the database name when it's supported?

I did not know that was the preference. My fault. Updated with the cluster parameter in the connection string, and retested each connection string to verify that that format indeed was supported.


v21.2/connection-reference.md, line 139 at r3 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Non-serverless clusters must also have a root certificate downloaded (in all languages, not just python and ruby).

We should probably have some explanation of what to do if you have to work with multiple clusters (or both cockroach and postgres). (You may be able to concatenate the files, or you may want to use multiple paths and specify them explicitly on the connection string depending on the security practices of the various CAs in question).

Good to note.

I envision this page having distinct serverless, dedicated, and self-hosted connection information for each tool, separated by functioning tabs. At this point, it just has info for serverless. After the structure of this page and of each tool's section is approved for serverless, I will add dedicated and core, which will include a more general note that dedicated and self-hosted clusters require a local root certificate.

I'll keep the commits separate so that reviewers can look at the changes individually after this first iteration is approved. I'll also have to test all of those snippets too, so it might take some time before I get those commits up.

Apologies for the churn. FWIW, I think getting the serverless snippets up and approved (at least in this PR) ASAP is going to help get design/product on the right track for the console updates more quickly.


v21.2/connection-reference.md, line 414 at r3 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

I think JDBC does support username and password in the connection string, it's just in a different place than the others. Append &user={user}&password={password}. (I think I've tested this in the past, but not today so please confirm).

Confirmed. Fixed.

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 246 at r3 (raw file):

This is misleading, because it's not the name of the cluster - it's the name of the cluster plus a unique number called the tenant_id. The problem is that two different customers could name their clusters the same thing, and we have to know which one to connect to. Therefore, we add a unique number that gives us that information.

Noted. I can make that change here, but I want to make sure that DX is on the same page. @kernfeld-cockroach I know that you had some misgivings about mentioning tenant ID in the docs (#12330 (comment) and https://cockroachlabs.atlassian.net/browse/DOC-1856).

I don't think we've been very consistent in our docs about how we refer to the clustername-tenantid value in general, but we should make a decision one way or another and aim for consistency. Because this page is a technical reference doc, we especially want to be accurate and clear on this page.

I've recently written an in-depth description of all the problems with connection routing. I'd suggest reading through that in order to get all the context of why the problem is so difficult to solve in a simple way.

Thanks for sharing! I did a quick read-through and found the motivation and implementation compelling.

@andy-kimball
Copy link

andy-kimball commented Dec 15, 2021


v21.2/connection-reference.md, line 246 at r3 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

This is misleading, because it's not the name of the cluster - it's the name of the cluster plus a unique number called the tenant_id. The problem is that two different customers could name their clusters the same thing, and we have to know which one to connect to. Therefore, we add a unique number that gives us that information.

Noted. I can make that change here, but I want to make sure that DX is on the same page. @kernfeld-cockroach I know that you had some misgivings about mentioning tenant ID in the docs (#12330 (comment) and https://cockroachlabs.atlassian.net/browse/DOC-1856).

I don't think we've been very consistent in our docs about how we refer to the clustername-tenantid value in general, but we should make a decision one way or another and aim for consistency. Because this page is a technical reference doc, we especially want to be accurate and clear on this page.

I've recently written an in-depth description of all the problems with connection routing. I'd suggest reading through that in order to get all the context of why the problem is so difficult to solve in a simple way.

Thanks for sharing! I did a quick read-through and found the motivation and implementation compelling.

I agree that we should not break out these different parts as in <cluster-name>-<tenant-id> for users. Instead, I think we should have a single term like routing-id and then tell customers to go look up their routing-id in the Connection Parameters section of their dashboard (similar to what @kernfeld-cockroach suggests). Today the routing ID will be <cluster-name>-<tenant-id>. If we adopt the proposal I linked to, then that will change to <tenant-id>.<host-short-id>. But in either case, we should not describe the component parts to the user. We should always treat it as an opaque routing-id, indivisible and not to be interpreted.

Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @ericharmeling, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 219 at r1 (raw file):

{{site.data.alerts.callout_info}}
JDBC connection URLs do not accept the `username` and `password` parameters. These parameters must be set outside of the connection string.

i believe a URL like this might work

jdbc:postgresql://{host}:{port}/{database}?sslmode=verify-full&options=--cluster%3D{cluster_name}&user={username}&password={password}&sslfactory=org.postgresql.ssl.DefaultJavaSSLFactory

v21.2/connection-reference.md, line 621 at r1 (raw file):

  host: "<host>",
  port: <port>,
  database: "<cluster_name>.<database>"

instead of using cluster.database here, i have tested sequelize and seen that it supports options like follows:

var sequelize = new Sequelize({
  dialect: "postgres",
  username: "<username>",
  password: "<password>",
  host: "<host>",
  port: <port>,
  database: "<database>",
  dialectOptions: {
    options: '--cluster=<cluster_name>'
  }
});

but this is only for sequelize v6.10.0 and greater. actually, the URL version also only is with v6.10.0 or greater. maybe that deserves a mention? otherwise you have to do some tricky stuff, like the "before" part of this commit: cockroachdb/examples-orms@7398e85


v21.2/connection-reference.md, line 668 at r1 (raw file):

    host: "<host>",
    port: <port>,
    database: "<cluster_name>.<database>"

i haven't tested typeorm myself, but based on my reading of the docs, this should work:

const connection = await createConnection({
    type: "postgresql",
    username: "<username>",
    password: "<password>",
    host: "<host>",
    port: <port>,
    database: "<database>",
    options: "--cluster=<cluster_name>"
});

v21.2/connection-reference.md, line 684 at r1 (raw file):

    "host": "<host>",
    "port": <port>,
    "database": "<cluster_name>.<database>"

ditto above comment about options

Copy link
Contributor

@rafiss rafiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops sorry my comments look out of place now since i left them on revision 1.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @ericharmeling, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 219 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i believe a URL like this might work

jdbc:postgresql://{host}:{port}/{database}?sslmode=verify-full&options=--cluster%3D{cluster_name}&user={username}&password={password}&sslfactory=org.postgresql.ssl.DefaultJavaSSLFactory

ah i see ben commented this on a later revision.

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR @rafiss !

oops sorry my comments look out of place now since i left them on revision 1.

No worries. I think I got to all of your comments that are still relevant to the latest revision, but some of the comments are now a little out-of-place in reviewable. LMK if I missed something.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @emily-horing, @ianjevans, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 621 at r1 (raw file):
I updated this snippet with the standard postgresql://<username>:<password>@<host>:<port>/<database>?sslmode=verify-full&options=--cluster%3D<routing-id> URL format - which I tested against v6.10 and v6.11.

actually, the URL version also only is with v6.10.0 or greater. maybe that deserves a mention?

I was actually able to get a URL to work for earlier versions, but with a slightly different format. I've added that format to the page in a new connection string snippet.

The page now lists a format for v6.10.0+ and a format for earlier versions. The earliest version I could get that second format to work with is 6.1.0 (which is like a year old at this point).

Note that the format for earlier versions appears to be backwards-compatible... so perhaps we just want to provide that one? It uses routing-id.database for the database name rather than the preferred cluster=routing-id individual parameter though, so IDK. I'm fine with providing both until we think most users will have 6.10+.


v21.2/connection-reference.md, line 668 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i haven't tested typeorm myself, but based on my reading of the docs, this should work:

const connection = await createConnection({
    type: "postgresql",
    username: "<username>",
    password: "<password>",
    host: "<host>",
    port: <port>,
    database: "<database>",
    options: "--cluster=<cluster_name>"
});

This config doesn't work for a few reasons:

  • Using the postgres type (postgresql isn't a valid type) returns some interesting errors that I don't really understand. Using cockroachdb as the type instead resolves those errors.
  • You need to specify a value for the ssl parameter (e.g., ssl: true). Not doing so results in the following error: error: server requires encryption.
  • options doesn't appear to be a valid config parameter for TypeORM. I think you have to use extras.

I updated this config to use a URL instead of individual parameters, but if we want to use the --cluster=<routing-id> option, we need to specify that parameter individually because TypeORM can't parse it from the URL (or at least I can't seem to format the URL in such a way that --cluster=<routing-id> gets parsed properly).

Also, I actually think it's much more common for users to use an ormconfig file for TypeORM (but I could be wrong on this?), and then just calling createConnection without any arguments (see https://typeorm.io/#/using-ormconfig/creating-a-new-connection-from-the-configuration-file). But the parameters should be the same.

I've updated the page with a working configuration - LMK if you think it needs any changes.


v21.2/connection-reference.md, line 684 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

ditto above comment about options

For ActiveRecord, I was able to get a pretty standard URL to work?

But you might be referring to a different section since the PR has been revised? I am guessing it was Django - which I updated accordingly to your suggestion. Thanks!


v21.2/connection-reference.md, line 579 at r2 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

Done. (went with Pool)

Actually, changing to use Client instead (seems like the most basic option).


v21.2/connection-reference.md, line 246 at r3 (raw file):

I agree that we should not break out these different parts as in - for users. Instead, I think we should have a single term like routing-id and then tell customers to go look up their routing-id in the Connection Parameters section

Okay, great. Sounds like everyone is on the same page then. Using a single term sounds like the more user-friendly, future-proof approach.

@emily-horing I'm going to use routing-id for now, but please LMK if there is a specific term we want to use. @ianjevans I imagine whatever term we use will also need to be reflected in docs changes for https://cockroachlabs.atlassian.net/browse/DOC-2040.

Copy link

@emily-horing emily-horing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @ianjevans, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 246 at r3 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

I agree that we should not break out these different parts as in - for users. Instead, I think we should have a single term like routing-id and then tell customers to go look up their routing-id in the Connection Parameters section

Okay, great. Sounds like everyone is on the same page then. Using a single term sounds like the more user-friendly, future-proof approach.

@emily-horing I'm going to use routing-id for now, but please LMK if there is a specific term we want to use. @ianjevans I imagine whatever term we use will also need to be reflected in docs changes for https://cockroachlabs.atlassian.net/browse/DOC-2040.

routing-id is ok with me for now. I think we'll want to reconsider the right convention once the changes Andy suggested are more finalized. (https://github.com/cockroachlabs/managed-service/pull/7411)

v21.2/connection-reference.md Outdated Show resolved Hide resolved
conn = psycopg.connect('{connection-string}')
~~~

Psycopg accepts the following format for CockroachDB connection strings:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was quite unsuccessfully attempting to articulate here is that, in an ideal world, we wouldn't even need to document the format of CockroachDB connection strings for Serverless because the user would simply copy the appropriate connection string directly from the CockroachDB Cloud web UI. However, I am now realizing that this is not immediately possible because the web UI doesn't generate the correct connection strings. So, I guess the first step would be for the web UI to generate the correct connection string for a particular tool. Once that's ready to go, then we could un-document this.

@andy-kimball
Copy link

I see that you switched the docs to use <routing-id>, but I don't see where that's defined or instructions on how to get it. Did I miss something or are you doing that in a different PR?

Copy link
Contributor

@kernfeld-cockroach kernfeld-cockroach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I learned a lot about connecting to CockroachDB!

~~~ js
const { Client } = require('pg')

const connectionString = '<connection-string>'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned that this might encourage developers who don't know any better to paste their connection strings into their source code, which is sketch because it contains the password. What do you think of modifying these examples to instead read from an environment variable named DATABASE_URL?

@kernfeld-cockroach
Copy link
Contributor

I think this information will be most useful for users who want to get started with an app. Can this go under "Get Started" instead?

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you switched the docs to use <routing-id>, but I don't see where that's defined or instructions on how to get it. Did I miss something or are you doing that in a different PR?

AFAIK, this is the first place where we use <routing-id> in the docs. As such, the only description of this term can be found in the "### Connection parameters" table(s) on this page. ATM, this description is very concise ("The routing ID of the CockroachDB cluster (e.g., funny-skunk-123)."). For users just copying and pasting from the console, IDK if they really need any more than this.

For users specifying parameters individually, I do think that providing this very simple example routing ID (funny-skunk-123) is helpful, but we ideally also want to send users to a page with more detailed instructions on how to get their cluster's routing ID (e.g., "For instructions on how to obtain your cluster's routing ID, see Connect to a CockroachDB Serverless (beta) Cluster").

@ianjevans any objections to me updating cockroachcloud/connect-to-a-serverless-cluster.html (and possibly other pages) to use <routing-id> instead of <cluster-name>-<tenant-id> in this PR? Or would you prefer to wait until after the Console changes have been made?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @emily-horing, @ianjevans, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 53 at r2 (raw file):

Previously, kernfeld-cockroach (Paul Kernfeld) wrote…

What I was quite unsuccessfully attempting to articulate here is that, in an ideal world, we wouldn't even need to document the format of CockroachDB connection strings for Serverless because the user would simply copy the appropriate connection string directly from the CockroachDB Cloud web UI. However, I am now realizing that this is not immediately possible because the web UI doesn't generate the correct connection strings. So, I guess the first step would be for the web UI to generate the correct connection string for a particular tool. Once that's ready to go, then we could un-document this.

For serverless, that does sound like an ideal scenario.

In the future, this page should serve as a reference for Self-hosted and Dedicated as well. As a result, we will want to keep the explicit connection string format docs for users that can't simply cut-and-paste from the console.


v21.2/connection-reference.md, line 54 at r3 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

I did not know that was the preference. My fault. Updated with the cluster parameter in the connection string, and retested each connection string to verify that that format indeed was supported.

@bdarnell Circling back on this ... Is there a specific reason why we prefer the cluster options parameter instead of putting the cluster name in the database name? (I assume that it is just more accurate/expected to have the routing ID in a separate parameter, especially for users specifying the individual parameters).

@emily-horing


v21.2/connection-reference.md, line 45 at r5 (raw file):

I'm concerned that this might encourage developers who don't know any better to paste their connection strings into their source code, which is sketch because it contains the password. What do you think of modifying these examples to instead read from an environment variable named DATABASE_URL?

That's a great idea. Done (and tested) for all except Django, which uses an individual configuration parameters.


v21.2/connection-reference.md, line 63 at r5 (raw file):

Previously, kernfeld-cockroach (Paul Kernfeld) wrote…

Do we want a ## Connect with Sequelize here?

Yes! Good catch. Also fixed this for TypeORM

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I learned a lot about connecting to CockroachDB!

Happy to hear that! TFTR!

I think this information will be most useful for users who want to get started with an app. Can this go under "Get Started" instead?

I discussed this today with @emily-horing, who suggested the same idea. I will discuss offline with @ianjevans to iron out details.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @emily-horing, @ianjevans, @jordanlewis, @kernfeld-cockroach, and @rafiss)

Copy link
Member

@bdarnell bdarnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell, @emily-horing, @ianjevans, @jordanlewis, @kernfeld-cockroach, and @rafiss)


v21.2/connection-reference.md, line 54 at r3 (raw file):

Previously, ericharmeling (Eric Harmeling) wrote…

@bdarnell Circling back on this ... Is there a specific reason why we prefer the cluster options parameter instead of putting the cluster name in the database name? (I assume that it is just more accurate/expected to have the routing ID in a separate parameter, especially for users specifying the individual parameters).

@emily-horing

Some tools get confused when the cluster name is added to the database name. I don't remember the specifics but for example I think one of them didn't like the fact that the database name in the connection string did not appear in SHOW DATABASES.

@ianjevans
Copy link
Contributor

Previously, ericharmeling (Eric Harmeling) wrote…
Some tools get confused when the cluster name is added to the database name. I don't remember the specifics but for example I think one of them didn't like the fact that the database name in the connection string did not appear in SHOW DATABASES.

Beekeeper Studio only partially works with Serverless clusters when connecting using the routing ID prefix. You can connect to a single database, and the other databases are listed in their UI, but you can't change databases. You have to disconnect, change the database name parameter, and reconnect. You can't use a connection string, as the options query parameter is ignored even if you modify it to use cockroachdb:// instead of postgresql://.

Navikat Premium doesn't work at all even though it appears to connect successfully to the cluster using the routing ID prefix (as reported in the community Slack channel, and I tested it out). No databases or schema objects show up in the UI.

@ericharmeling ericharmeling force-pushed the connection-reference branch 3 times, most recently from 1a45b58 to 17ff9f6 Compare January 7, 2022 15:58
@ericharmeling
Copy link
Contributor Author

I think that I've addressed everyone's initial feedback.

Here are some of the biggest changes that I've made in the last several commits:

  • Filled out the Dedicated connection tab with tested connection string formats. Dedicated connection strings seem to be a lot more straightforward. I think I'm going to leave self-hosted out of this PR for now, so we can get this out ASAP.
  • Moved this page from "Reference" to "Develop"/"Connect to CockroachDB", following the recent docs nav restructure.
  • Updated docs to use routing-id instead of cluster-name.tenant-id, and added more context on "routing ID" terminology.

Please let me know if there are any outstanding comments/concerns. If not, I'll move this to writer review early next week (Monday evening or Tuesday morning).

Note: Since returning from PTO 1/4/22, I was unable to get Sequelize connection strings to work at all, I believe due to cockroachdb/sequelize-cockroachdb#141 / cockroachdb/cockroach#74057.

Copy link
Contributor

@ianjevans ianjevans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The links to the Cloud architecture doc were showing up as external. I added suggestions you can batch to fix the links.

v21.2/connect-to-the-database.md Outdated Show resolved Hide resolved
v21.2/connect-to-the-database.md Outdated Show resolved Hide resolved
cockroachcloud/connect-to-a-serverless-cluster.md Outdated Show resolved Hide resolved
v21.2/build-a-csharp-app-with-cockroachdb.md Outdated Show resolved Hide resolved
v21.2/connect-to-the-database.md Outdated Show resolved Hide resolved
v21.2/connect-to-the-database.md Outdated Show resolved Hide resolved
v21.2/connect-to-the-database.md Outdated Show resolved Hide resolved
v21.2/connection-parameters.md Outdated Show resolved Hide resolved
v21.2/hello-world-python-sqlalchemy.md Outdated Show resolved Hide resolved
@emily-horing
Copy link

One thing that came up when I was reviewing our latest mocks with Rachel I wanted to bring up here: is it confusing that we're now calling it "routingID" but the actual text says --options cluster=<clustername+tenantid>? It seems pretty clear that this is the cluster, and there's no reference to routing here.

Made me wonder how trivial it might be to add a second --options path that used routing=<clustername+tenantid>, which we could update to show in the UI without impacting any existing connections. cc @andy-kimball for thoughts on that.

@ericharmeling I'd be curious (assuming we keep everything the same) what you would suggest we call this in the parameters dialog in the UI. This would likely be used mostly to connect to tools like DataGrip (see this issue for an example Ian helped solve for). Feel free to leave comments in the Figma!

@andy-kimball
Copy link

Made me wonder how trivial it might be to add a second --options path that used routing=<clustername+tenantid>

That would be very easy to do. Can you file a JIRA issue, and we can find someone to do that work?

@ericharmeling
Copy link
Contributor Author

ericharmeling commented Jan 10, 2022

Made me wonder how trivial it might be to add a second --options path that used routing=<clustername+tenantid>, which we could update to show in the UI without impacting any existing connections.

Great idea. We might want to document those changes (once implemented) as part of a follow-up docs PR though.

I'd be curious (assuming we keep everything the same) what you would suggest we call this in the parameters dialog in the UI.

We could change the terminology in the docs and in the parameters dialog to be "cluster routing ID" (with placeholder cluster-routing-id). This would still be accurate should the format of the actual routing ID change, and it includes both the words "cluster" and "routing", which would be consistent with both --options cluster= and --options routing=. Thoughts?

@ericharmeling
Copy link
Contributor Author

ericharmeling commented Jan 10, 2022

The links to the Cloud architecture doc were showing up as external. I added suggestions you can batch to fix the links.

TFTR! Fixed

@ericharmeling ericharmeling force-pushed the connection-reference branch 2 times, most recently from 2ee969e to dc1c256 Compare January 10, 2022 14:26
Copy link
Contributor

@nickvigilante nickvigilante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericharmeling Took a look at it and this was an impressive feat to accomplish. Just a few nitpicky things here and there and some broken links, but otherwise, LGTM!

@@ -51,7 +52,7 @@ A growing number of popular third-party database tools offer full support for Co

## What's next?

- [Start a Local Cluster](secure-a-cluster.html)
- [Create a CockroachDB Serverless Cluster](../cockroachcloud/create-a-serverless-cluster.html)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericharmeling Can you use the liquid tag {{ site.data.products.serverless }} instead of outright saying "CockroachDB Serverless" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -67,7 +67,7 @@ Scaling application deployments in multiple regions can greatly improve latency

For guidance on connecting to CockroachDB from an application deployment, see one of the following pages:

- For connecting to managed, {{ site.data.products.db }} deployments, see [Connect to Your {{ site.data.products.dedicated }} Cluster](../cockroachcloud/connect-to-your-cluster.html) and [Connect to the Database ({{ site.data.products.dedicated }})](connect-to-the-database-cockroachcloud.html).
- For connecting to managed, {{ site.data.products.db }} deployments, see [Connect to Your {{ site.data.products.dedicated }} Cluster](../cockroachcloud/connect-to-your-cluster.html) and [Connect to the Database ({{ site.data.products.dedicated }})](connect-to-the-database.html).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericharmeling In the URL for connect-to-the-database.html, can you append the following?

?filters=dedicated

That way, when users click that link, it automatically opens the tab that says Dedicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<div class="filter-content" markdown="1" data-scope="serverless">

{{site.data.alerts.callout_info}}
To connect to a {{ site.data.products.serverless-plan }} cluster from a Python application, you must have a valid CA certificate located at <code>~/.postgresql/root.crt</code>.<br>For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see <a href="https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html">Connect to a {{ site.data.products.serverless-plan }} Cluster</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raw and broken hyperlink. Replace this:

https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html

with this:

../cockroachcloud/connect-to-a-serverless-cluster.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

</div>

{{site.data.alerts.callout_info}}
To connect to CockroachDB with Hibernate, you must specify the [CockroachDB Hibernate dialect](https://www.cockroachlabs.com/docs/v21.2/install-client-drivers?filters=java#hibernate) in your `hibernate.cfg.xml` configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this:

https://www.cockroachlabs.com/docs/v21.2/install-client-drivers?filters=java#hibernate

with this:

{{ page.version.version }}/install-client-drivers?filters=java#hibernate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or better yet, just do away completely with the page.version.version prefix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

</div>

{{site.data.alerts.callout_info}}
To connect to a {{ site.data.products.serverless-plan }} cluster from a Ruby application, you must have a valid CA certificate located at <code>~/.postgresql/root.crt</code>.<br>For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see <a href="https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html">Connect to a {{ site.data.products.serverless-plan }} Cluster</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

../cockroachcloud/connect-to-a-serverless-cluster.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

For a list of community-supported third-party tools, see [Third-Party Tools Supported by the Community](community-tooling.html). CockroachDB supports both native drivers and the PostgreSQL wire protocol. Most client drivers and ORM frameworks connect to CockroachDB like they connect to PostgreSQL.

<div class="filters clearfix">
<button class="filter-button page-level" data-scope="serverless">{{ site.data.products.serverless-plan }}</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason we're using {{ site.data.products.serverless-plan }} and not {{ site.data.products.serverless }}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{ site.data.products.serverless-plan }} maps to "CockroachDB Serverless" and {{ site.data.products.serverless }} maps to "CockroachDB Serverless (beta)"

While more accurate, "CockroachDB Serverless (beta)" is a little unnecessarily bulky to be throwing around every time we mention serverless (especially in things like tab titles). But that's just my opinion.

@ianjevans Is there a set convention for when to use {{ site.data.products.serverless-plan }} vs {{ site.data.products.serverless}}?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent was to separate the name of the product from what type of cluster the user would see in the Cloud UI. But in practice it has turned into a short name for the product.

The problem with that is eventually we're going to drop the "(beta)" from the product name and the two variables are going to be identical.

Now I'm wondering if we really want "(beta)" there at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianjevans I think we want the beta there for now while the product is still in beta, but the nice thing is once it's out of beta, we just remove 7 characters from 1 file and it gets updated across the board.

Like I wrote in Slack, I think we should stick with just serverless and repurpose serverless-plan for what it was originally intended to be, which I still don't fully understand what that is 😄

As bulky as it is, we will eventually move out of beta, and if for some reason the product name changes again, at least we'll be able to update it rather effortlessly. Regardless, we're going to have to do a universal find and replace of serverless-plan if we've been using them interchangeably, so probably keeping it for now will not make much of a difference in the end.

- Removed all root certificate parameters
- Fixed syntax errors
- Updated misc configuration settings
- Reordered by preferred language
- Simplified all sections to use a single connection methodology
- Fixed formatting errors
- Simplified Java connection strings
- Updated all connection URLs to use cluster options parameter
- Changed cluster-name to routing-id
- Updated TypeORM, Django configurations
Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nickvigilante TFTR! Much appreciated. I think I got to all your comments.

For a list of community-supported third-party tools, see [Third-Party Tools Supported by the Community](community-tooling.html). CockroachDB supports both native drivers and the PostgreSQL wire protocol. Most client drivers and ORM frameworks connect to CockroachDB like they connect to PostgreSQL.

<div class="filters clearfix">
<button class="filter-button page-level" data-scope="serverless">{{ site.data.products.serverless-plan }}</button>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{ site.data.products.serverless-plan }} maps to "CockroachDB Serverless" and {{ site.data.products.serverless }} maps to "CockroachDB Serverless (beta)"

While more accurate, "CockroachDB Serverless (beta)" is a little unnecessarily bulky to be throwing around every time we mention serverless (especially in things like tab titles). But that's just my opinion.

@ianjevans Is there a set convention for when to use {{ site.data.products.serverless-plan }} vs {{ site.data.products.serverless}}?

<div class="filter-content" markdown="1" data-scope="serverless">

{{site.data.alerts.callout_info}}
To connect to a {{ site.data.products.serverless-plan }} cluster from a Python application, you must have a valid CA certificate located at <code>~/.postgresql/root.crt</code>.<br>For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see <a href="https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html">Connect to a {{ site.data.products.serverless-plan }} Cluster</a>.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

</div>

{{site.data.alerts.callout_info}}
To connect to CockroachDB with Hibernate, you must specify the [CockroachDB Hibernate dialect](https://www.cockroachlabs.com/docs/v21.2/install-client-drivers?filters=java#hibernate) in your `hibernate.cfg.xml` configuration file.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

</div>

{{site.data.alerts.callout_info}}
To connect to a {{ site.data.products.serverless-plan }} cluster from a Ruby application, you must have a valid CA certificate located at <code>~/.postgresql/root.crt</code>.<br>For instructions on downloading a CA certificate from the {{ site.data.products.db }} Console, see <a href="https://www.cockroachlabs.com/docs/v21.2/cockroachcloud/connect-to-a-serverless-cluster.html">Connect to a {{ site.data.products.serverless-plan }} Cluster</a>.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -51,7 +52,7 @@ A growing number of popular third-party database tools offer full support for Co

## What's next?

- [Start a Local Cluster](secure-a-cluster.html)
- [Create a CockroachDB Serverless Cluster](../cockroachcloud/create-a-serverless-cluster.html)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -67,7 +67,7 @@ Scaling application deployments in multiple regions can greatly improve latency

For guidance on connecting to CockroachDB from an application deployment, see one of the following pages:

- For connecting to managed, {{ site.data.products.db }} deployments, see [Connect to Your {{ site.data.products.dedicated }} Cluster](../cockroachcloud/connect-to-your-cluster.html) and [Connect to the Database ({{ site.data.products.dedicated }})](connect-to-the-database-cockroachcloud.html).
- For connecting to managed, {{ site.data.products.db }} deployments, see [Connect to Your {{ site.data.products.dedicated }} Cluster](../cockroachcloud/connect-to-your-cluster.html) and [Connect to the Database ({{ site.data.products.dedicated }})](connect-to-the-database.html).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@nickvigilante nickvigilante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Add per-driver connection information
9 participants