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

Connect: Fix and improve connection strings #151

Merged
merged 6 commits into from
Nov 22, 2024
Merged

Conversation

amotl
Copy link
Member

@amotl amotl commented Nov 15, 2024

About

A few fixes/improvements to different connection strings we are presenting on the documentation. For example:

  • Add missing ssl=true argument on SQLAlchemy connection string.
  • Remove /<dbname> argument from SQLAlchemy connection string, because it would cause errors.

Preview

https://crate-clients-tools--151.org.readthedocs.build/en/151/connect/

/cc @surister, @simonprickett

@amotl amotl requested review from kneth and proddata November 15, 2024 20:53
@cla-bot cla-bot bot added the cla-signed label Nov 15, 2024
@amotl amotl force-pushed the amo/connect-ssl-true branch from d4b7fb6 to 2b82c00 Compare November 15, 2024 21:04
@amotl amotl changed the title Connect: Add missing ssl=true argument on SQLAlchemy connection string Connect: Fix and improve connection strings Nov 15, 2024
docs/connect/index.md Outdated Show resolved Hide resolved
@@ -101,10 +101,10 @@ An HTTP URL to visit Admin UI.
**Connection-string examples**

A native PostgreSQL connection string.
`postgresql://crate@localhost:5432/crate`
Copy link
Member

Choose a reason for hiding this comment

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

technically /crate specifies the database to be used.
Unfortunately CrateDB still uses them in a non pg-compliant way
crate/crate#13502

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think it is a good measure to slap the "Blocked by CrateDB" label onto the root issue, in order to increase awareness?

image

@kneth kneth removed their request for review November 20, 2024 09:55
@kneth
Copy link

kneth commented Nov 20, 2024

I have removed me as reviewer as my SQL Alchemy and Apache Flink knowledge is too shallow

@amotl amotl force-pushed the amo/connect-ssl-true branch from 2b82c00 to 7f16fd1 Compare November 21, 2024 00:48
@amotl amotl requested review from surister and proddata November 21, 2024 00:49
docs/connect/index.md Outdated Show resolved Hide resolved
docs/connect/index.md Outdated Show resolved Hide resolved
@amotl
Copy link
Member Author

amotl commented Nov 22, 2024

I think this yak shaving went well? Let me know if you see any spots for improvements, or if you would do it completely different.

image
-- https://crate-clients-tools--151.org.readthedocs.build/en/151/connect/

Please check, also @karynzv, @hammerhead, @wierdvanderhaar, @zolbatar, @lservini, if you can spot something to be improved in this section of the documentation, also in general beyond this widget. Thank you.

NB: Dark mode courtesy of Furo and @msbt. 🚀

@amotl amotl force-pushed the amo/connect-ssl-true branch from 728947a to 418d3d6 Compare November 22, 2024 15:38
CrateDB is different in different aspects regarding the semantics of
common database servers, because it does not implement "databases".

This section aims to educate about the differences in a very concise
manner, including relevant hints that might save users from too many
permutations in probing failed connection attempts.
@amotl amotl force-pushed the amo/connect-ssl-true branch from 418d3d6 to 8cd4341 Compare November 22, 2024 15:46
Copy link
Member Author

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Yet another iteration on the nitty gritty details, in order to not let it slip into the unknown future.

Comment on lines 169 to 181
```{tip}
- Specify the [schema] name `doc`, if you are asked for a *database name*.
- The default [superuser] on a vanilla CrateDB cluster is called `crate`,
without a password.
- When aiming to authenticate properly, please learn about the available
[authentication] options.
- CrateDB's fixed catalog name is `crate`, the default schema name is `doc`.
- CrateDB does not implement the notion of a database,
however tables can be created in different [schemas].
- When asked for a *database name*, specifying a schema name (any),
or the fixed catalog name `crate` may be applicable.
- If a database-/schema-name is omitted while connecting,
the PostgreSQL drivers may default to the "username".
- The predefined [superuser] on an unconfigured CrateDB cluster is
called `crate`, defined without a password.
- For authenticating properly, please learn about the available
[authentication] options.
```
Copy link
Member Author

@amotl amotl Nov 22, 2024

Choose a reason for hiding this comment

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

Reply

Tools like DBeaver will default to your username as "database" name. In CrateDB this currently sets your searchpath to username. Do you want that?

This is certainly not intended, [...] improving the documentation [makes sense].

Thanks again. Within the "Tip" section below, I've added this item:

  • If a database-/schema-name is omitted while connecting,
    the PostgreSQL drivers may default to the "username".

Report

There have been a few more updates to this section, aiming to compress relevant details about those matters into a single spot.

image
-- Connect » Configure

Copy link
Member Author

@amotl amotl Nov 22, 2024

Choose a reason for hiding this comment

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

^^

Connect: Improve "Tip" section about specifics of catalog v db v schema

CrateDB is different in different aspects regarding the semantics of
common database servers, because it does not implement "databases".

This section aims to educate about the differences in a very concise
manner, including relevant hints that might save users from too many
permutations in probing failed connection attempts.

-- 8cd4341

We compiled those details based on a few empirical exercises, and your advises. Please validate.

Please let us know if you can think of anything missing in this summary, or if existing items need to improve in wording and overall clarity.

Copy link
Member Author

@amotl amotl Nov 22, 2024

Choose a reason for hiding this comment

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

I guess it will not harm to merge this, now after refining it on behalf of multiple iterations. Please let me know about any improvements and wishes also in retrospective. Thanks for your valuable support so far!

@amotl amotl merged commit 2295700 into main Nov 22, 2024
4 checks passed
@amotl amotl deleted the amo/connect-ssl-true branch November 22, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants