-
Notifications
You must be signed in to change notification settings - Fork 1
Driver: Improve section about Java #402
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughRemoves the legacy consolidated Java JDBC page, adds a reorganized Java connect section with multiple new pages (separate PostgreSQL and CrateDB JDBC guides, jOOQ, Hibernate/JPA, testing, and a JDBC example), updates connect navigation to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
b8498e4
to
494a5ae
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/connect/java/index.md
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-25T19:31:54.320Z
Learnt from: bmunkholm
PR: crate/cratedb-guide#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.
Applied to files:
docs/connect/java/index.md
📚 Learning: 2025-10-08T01:34:18.867Z
Learnt from: amotl
PR: crate/cratedb-guide#385
File: docs/connect/java.md:48-51
Timestamp: 2025-10-08T01:34:18.867Z
Learning: CrateDB JDBC driver uses the `jdbc:crate://` protocol scheme but communicates via the PostgreSQL wire protocol on port 5432, just like the PostgreSQL JDBC driver (`jdbc:postgresql://`). Do not confuse the `jdbc:crate://` scheme with other protocol schemes like `crate://` (used by SQLAlchemy dialect for CrateDB) or assume it uses the HTTP endpoint on port 4200.
Applied to files:
docs/connect/java/index.md
📚 Learning: 2025-08-05T07:14:57.416Z
Learnt from: hammerhead
PR: crate/cratedb-guide#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/java/index.md
🪛 LanguageTool
docs/connect/java/index.md
[style] ~74-~74: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 2173 characters long)
Context: ...OOQ example. :::: ::::{grid-item-card} ![JUnit logo][JUnit logo]{height=40px} &n...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/connect/java/index.md
46-46: Reference links and images should use a label that is defined
Missing link or image reference definition: "postgresql logo"
(MD052, reference-links-images)
53-53: Reference links and images should use a label that is defined
Missing link or image reference definition: "cratedb logo"
(MD052, reference-links-images)
60-60: Reference links and images should use a label that is defined
Missing link or image reference definition: "hibernate logo"
(MD052, reference-links-images)
67-67: Reference links and images should use a label that is defined
Missing link or image reference definition: "jooq logo"
(MD052, reference-links-images)
74-74: Reference links and images should use a label that is defined
Missing link or image reference definition: "junit logo"
(MD052, reference-links-images)
You can also talk to CrateDB using HTTP, using any HTTP library of your choice. | ||
Below is a quick example using Apache Commons HTTP. | ||
|
||
:::{todo} | ||
Add example. | ||
::: |
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.
Provide the promised HTTP example instead of a TODO block.
Line 86 promises “Below is a quick example using Apache Commons HTTP,” but the section only contains a todo
directive. Readers will either see an empty promise or the literal TODO, which undercuts the usefulness of this page. Please add the example (or remove the teaser sentence until the example is ready) before merging.
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.
Ack. If we can't add an example before review, we will remove the section before merging.
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.
Thx, left some suggestions.
Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
``` | ||
::: | ||
Invoke program. Needs Java >= 25 ([JEP 330]). |
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.
crate-jdbc requires >= 11 (https://github.com/crate/crate-jdbc/blob/master/build.gradle#L19). Also the given command does not required JEP 330 afaik.
Invoke program. Needs Java >= 25 ([JEP 330]). | |
Invoke program. Needs Java >= 11. |
- {ref}`postgresql-jdbc` — `jdbc:postgresql://` | ||
- {ref}`cratedb-jdbc` — `jdbc:crate://` | ||
|
||
Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath |
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.
Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath | |
Prefer the PostgreSQL JDBC driver first, it’s often already on your classpath |
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.
also:
Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath | |
Prefer the PostgreSQL JDBC driver first—it’s often already in your classpath |
Invoke-WebRequest https://repo1.maven.org/maven2/org/postgresql/postgresql/42.7.8/postgresql-42.7.8.jar -OutFile postgresql-42.7.8.jar | ||
``` | ||
::: | ||
Invoke program. Needs Java >= 25 ([JEP 330]). |
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.
PostgreSQL-JDBC works with Java >= 1.8, and the given command does not need JEP 330 afaik.
Invoke program. Needs Java >= 25 ([JEP 330]). | |
Invoke program. Needs Java >= 1.8. |
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.
Thx, Don't have more comments myself.
About
Preview
https://cratedb-guide--402.org.readthedocs.build/connect/java/
/cc @zolbatar, @kneth, @surister