Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughAdds documentation for a Zig CrateDB driver: a new driver page with examples and build snippets, updates the main Connect drivers index to include a Zig card and navigation entry, and appends an OpenTelemetry URL to docs linkcheck_ignore. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
4722c90 to
12be5ca
Compare
kneth
left a comment
There was a problem hiding this comment.
Let's discuss https://github.com/crate/tech-content/issues/124#issuecomment-3485676563 before merging
|
I think it's nice to have a dedicated page about Zig, so that all downstream entities (humans and machines) can easily learn how to connect to CrateDB using the PostgreSQL interface, and use a ready-to-run example to exercise it right away. In the long run, this will also improve network effects, because CrateDB gains a dedicated representation within the corresponding search- and vectorspaces. Currently, relevant search keywords like cratedb zig will not yield concise results, but this will improve a few days after merging this patch. |
matriv
left a comment
There was a problem hiding this comment.
left a couple of comments, thx!
| const exe = b.addExecutable(.{ | ||
| .name = "example", | ||
| .root_module = b.createModule(.{ | ||
| .root_source_file = b.path("example.zig"), |
There was a problem hiding this comment.
what is this file: example.zig? I don't think it's explained.
There was a problem hiding this comment.
It is the program right below the build script.
There was a problem hiding this comment.
I think we should clarify this.
docs/connect/zig/index.md
Outdated
| :::{rubric} CrateDB Cloud | ||
| ::: | ||
|
|
||
| For connecting to CrateDB Cloud, use the `sslmode=require` parameter, |
There was a problem hiding this comment.
| For connecting to CrateDB Cloud, use the `sslmode=require` parameter, | |
| For connecting to a CrateDB node with ssl enabled (e.g.: CrateDB Cloud), use the `sslmode=require` parameter, |
Maybe this wording is better to reveal more generic usage?
There was a problem hiding this comment.
Right, we even renamed the headline on previous patches. 044df39 does the same here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/connect/zig/index.md`:
- Around line 43-44: Add a one-line clarification after the `example.zig`
filename stating that `example.zig` is the program produced by running
`build.zig`; reference both filenames (`example.zig`, `build.zig`) so readers
understand that `build.zig` builds the `example.zig` executable.
- Around line 55-56: Update the example connection URI to use the default empty
local password: change the const uri assignment that calls std.Uri.parse
(currently "postgresql://crate:crate@localhost/?sslmode=disable") to omit the
password so the URI becomes "postgresql://crate@localhost/?sslmode=disable"
before calling pg.Pool.initUri with allocator and pool.
🧹 Nitpick comments (2)
docs/connect/zig/index.md (2)
23-31: Honor the selected target in the build example.Using
b.graph.hostignoresb.standardTargetOptions, so cross-compiles won’t follow the user-selected target. Consider wiring.target = targetinstead.♻️ Proposed fix
- .root_module = b.createModule(.{ - .root_source_file = b.path("example.zig"), - .target = b.graph.host, - }) + .root_module = b.createModule(.{ + .root_source_file = b.path("example.zig"), + .target = target, + })
73-75: Pinpg.zigto a tag/commit instead of#master.Using
#masteris non-reproducible and may break as upstream changes. Prefer a release tag or commit hash.♻️ Example pin
-zig fetch --save git+https://github.com/karlseguin/pg.zig#master +zig fetch --save git+https://github.com/karlseguin/pg.zig#vX.Y.Z
| const uri = try std.Uri.parse("postgresql://crate:crate@localhost/?sslmode=disable"); | ||
| var pool = try pg.Pool.initUri(allocator, uri, .{.size=5, .timeout=5_000}); |
There was a problem hiding this comment.
Use default credentials in the local URI.
The example uses a non-default password (crate), which breaks the out-of-the-box local setup where password is empty. Please align with default connection parameters. Based on learnings, ...
✅ Proposed fix
- const uri = try std.Uri.parse("postgresql://crate:crate@localhost/?sslmode=disable");
+ const uri = try std.Uri.parse("postgresql://crate@localhost/?sslmode=disable");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const uri = try std.Uri.parse("postgresql://crate:crate@localhost/?sslmode=disable"); | |
| var pool = try pg.Pool.initUri(allocator, uri, .{.size=5, .timeout=5_000}); | |
| const uri = try std.Uri.parse("postgresql://crate@localhost/?sslmode=disable"); | |
| var pool = try pg.Pool.initUri(allocator, uri, .{.size=5, .timeout=5_000}); |
🤖 Prompt for AI Agents
In `@docs/connect/zig/index.md` around lines 55 - 56, Update the example
connection URI to use the default empty local password: change the const uri
assignment that calls std.Uri.parse (currently
"postgresql://crate:crate@localhost/?sslmode=disable") to omit the password so
the URI becomes "postgresql://crate@localhost/?sslmode=disable" before calling
pg.Pool.initUri with allocator and pool.
| b.installArtifact(exe); | ||
| } | ||
| ``` | ||
| `example.zig` |
There was a problem hiding this comment.
I would define this first, and then the above code which uses it.
About
What the title says.
Preview
https://cratedb-guide--419.org.readthedocs.build/connect/zig/