-
Notifications
You must be signed in to change notification settings - Fork 1
Driver: Fix "SSL connection" section on "Erlang/ODBC" page #474
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
Conversation
WalkthroughReplaced "CrateDB Cloud" heading with "SSL connection", revised intro to instruct starting the Erlang SSL application and using Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant ErlangVM as Erlang VM
participant SSL as ssl
participant ODBC as odbc
participant CrateDB as CrateDB (testcluster.cratedb.net)
Dev->>ErlangVM: run example
ErlangVM->>SSL: ssl:start()
SSL-->>ErlangVM: SSL ready
ErlangVM->>ODBC: odbc:start()
ODBC-->>ErlangVM: ODBC ready
ErlangVM->>CrateDB: connect(Server=testcluster.cratedb.net, Sslmode=require, Uid, Pwd)
CrateDB-->>ErlangVM: TLS handshake / connection established
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cafac82 to
845edcc
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/connect/erlang/odbc.md (2)
39-46: Clarify the purpose of SSL connection to reduce redundancy.Lines 42–44 already state "For connecting to CrateDB Cloud," and line 46 reiterates "Also use this variant to connect to CrateDB Cloud." This creates redundancy and may inadvertently imply SSL is primarily for CrateDB Cloud rather than a general technique.
Consider either:
- Removing line 46 since it's already covered, or
- Reframing the intro text to emphasize SSL as a general connection method, with CrateDB Cloud as one specific application.
For example: "To enable SSL connections, start the Erlang [SSL application], add
sslmode=require, and adjustServer,Uid, andPwdfor your environment (e.g., CrateDB Cloud)."
49-58: Optional: Consider aligning the example with established documentation patterns.Per the learning from PR #420, cratedb-guide typically shows only the differences from the canonical example when documenting alternative connection methods (e.g., SSL configuration), rather than repeating the entire workflow. The current example duplicates the full workflow (lines 29–37 vs. lines 49–58).
For a more concise approach, you could show only the SSL-specific setup:
ssl:start(), odbc:start(), {ok, Ref} = odbc:connect("Driver={PostgreSQL Unicode};Server=testcluster.cratedb.net;Port=5432;sslmode=require;Uid=admin;Pwd=password", []),That said, if full examples improve clarity for this section, this is optional. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/connect/erlang/odbc.md(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-guide PR: 420
File: docs/connect/erlang.md:100-113
Timestamp: 2025-10-19T19:21:49.864Z
Learning: In the cratedb-guide repository, when documenting cloud or alternative connection methods, show only the differences from the canonical example (e.g., SSL configuration) rather than repeating the entire workflow (query execution, cleanup, etc.). This pattern is used consistently across pages to avoid repetition.
Learnt from: amotl
Repo: crate/cratedb-guide PR: 406
File: docs/connect/go.md:110-137
Timestamp: 2025-10-20T21:49:18.785Z
Learning: In the cratedb-guide repository, documentation examples for connecting to CrateDB (e.g., Go examples in docs/connect/go.md) should be kept minimal and focused on demonstrating basic functionality, rather than including comprehensive error handling. Full examples with proper error handling are maintained separately in the cratedb-examples repository.
Learnt from: amotl
Repo: crate/cratedb-guide PR: 408
File: docs/connect/elixir.md:18-22
Timestamp: 2025-10-16T10:58:18.401Z
Learning: When connecting to CrateDB using PostgreSQL-compatible drivers like Postgrex (Elixir), the `database` parameter handling may differ from standard PostgreSQL usage because CrateDB has no notion of databases like PostgreSQL does. CrateDB uses schemas to separate concerns instead. The `database` parameter might need to be omitted or handled differently to avoid connection issues.
Learnt from: amotl
Repo: crate/cratedb-guide PR: 253
File: docs/integrate/mindsdb/index.md:21-33
Timestamp: 2025-08-14T19:02:43.308Z
Learning: In CrateDB integration documentation examples, default connection parameters (user="crate", password="", host="127.0.0.1") are intentionally used to provide working out-of-the-box examples for users with local CrateDB instances, rather than using placeholder values that require customization.
Learnt from: bmunkholm
Repo: crate/cratedb-guide PR: 340
File: docs/home/index.md:84-97
Timestamp: 2025-09-25T19:31:54.320Z
Learning: In the CrateDB Guide docs (MyST), the CrateDB Cloud card on the homepage should link to `getting-started` using `:link-type: ref` instead of the previous `cloud:index` intersphinx target. This change was implemented in PR #340 to direct users to the getting started section rather than directly to the Cloud documentation.
Learnt from: amotl
Repo: crate/cratedb-guide PR: 207
File: docs/integrate/etl/iceberg-risingwave.md:205-207
Timestamp: 2025-06-05T14:29:15.512Z
Learning: The `records.Database("crate://", echo=True)` connection string for CrateDB works with defaults: localhost as host, "crate" as user, and blank password. This is valid and functional code in the records library.
Learnt from: amotl
Repo: crate/cratedb-guide PR: 248
File: docs/integrate/amqp/tutorial.md:36-41
Timestamp: 2025-08-14T12:42:12.744Z
Learning: In the cratedb-guide repository, code examples and tutorials prioritize simplicity and ease of following over security hardening. Security recommendations should be more lenient in tutorial/demo contexts to avoid distracting from the main learning objectives.
Learnt from: hammerhead
Repo: crate/cratedb-guide PR: 221
File: docs/connect/configure.md:58-66
Timestamp: 2025-08-05T07:14:57.416Z
Learning: In CrateDB connection strings, the user:password@ syntax is valid for HTTP Basic authentication on port 4200, but PostgreSQL JDBC drivers do not support this format and require credentials as query parameters (?user=<user>&password=<password>) instead.
📚 Learning: 2025-10-19T19:21:49.864Z
Learnt from: amotl
Repo: crate/cratedb-guide PR: 420
File: docs/connect/erlang.md:100-113
Timestamp: 2025-10-19T19:21:49.864Z
Learning: In the cratedb-guide repository, when documenting cloud or alternative connection methods, show only the differences from the canonical example (e.g., SSL configuration) rather than repeating the entire workflow (query execution, cleanup, etc.). This pattern is used consistently across pages to avoid repetition.
Applied to files:
docs/connect/erlang/odbc.md
📚 Learning: 2025-10-16T10:58:18.401Z
Learnt from: amotl
Repo: crate/cratedb-guide PR: 408
File: docs/connect/elixir.md:18-22
Timestamp: 2025-10-16T10:58:18.401Z
Learning: When connecting to CrateDB using PostgreSQL-compatible drivers like Postgrex (Elixir), the `database` parameter handling may differ from standard PostgreSQL usage because CrateDB has no notion of databases like PostgreSQL does. CrateDB uses schemas to separate concerns instead. The `database` parameter might need to be omitted or handled differently to avoid connection issues.
Applied to files:
docs/connect/erlang/odbc.md
📚 Learning: 2025-08-14T19:02:43.308Z
Learnt from: amotl
Repo: crate/cratedb-guide PR: 253
File: docs/integrate/mindsdb/index.md:21-33
Timestamp: 2025-08-14T19:02:43.308Z
Learning: In CrateDB integration documentation examples, default connection parameters (user="crate", password="", host="127.0.0.1") are intentionally used to provide working out-of-the-box examples for users with local CrateDB instances, rather than using placeholder values that require customization.
Applied to files:
docs/connect/erlang/odbc.md
📚 Learning: 2025-06-05T14:29:15.512Z
Learnt from: amotl
Repo: crate/cratedb-guide PR: 207
File: docs/integrate/etl/iceberg-risingwave.md:205-207
Timestamp: 2025-06-05T14:29:15.512Z
Learning: The `records.Database("crate://", echo=True)` connection string for CrateDB works with defaults: localhost as host, "crate" as user, and blank password. This is valid and functional code in the records library.
Applied to files:
docs/connect/erlang/odbc.md
📚 Learning: 2025-08-05T07:14:57.416Z
Learnt from: hammerhead
Repo: crate/cratedb-guide PR: 221
File: docs/connect/configure.md:58-66
Timestamp: 2025-08-05T07:14:57.416Z
Learning: In CrateDB connection strings, the user:password@ syntax is valid for HTTP Basic authentication on port 4200, but PostgreSQL JDBC drivers do not support this format and require credentials as query parameters (?user=<user>&password=<password>) instead.
Applied to files:
docs/connect/erlang/odbc.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build docs
845edcc to
02f5893
Compare
About
Just fixing a missing detail. Thanks @matriv.
Reference