-
Notifications
You must be signed in to change notification settings - Fork 9
ROAPI: Basic example (mount CSV, JSON, and Parquet) #1286
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
WalkthroughAdds a ROAPI + CrateDB integration: GitHub Actions workflow, Docker Compose orchestration, multi-stage ROAPI Dockerfile, orchestration/test script, README and notes documenting data registration and querying via a CrateDB FDW. (45 words) Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant CI as test.sh (runner)
participant DC as Docker Compose
participant RO as ROAPI
participant CR as CrateDB
participant TASK as one‑off tasks
GH->>CI: workflow triggered (push/PR/dispatch/schedule)
CI->>DC: docker compose up --build --detach (cratedb, roapi)
DC->>CR: start -> pass healthcheck
DC->>RO: start -> service_started
CI->>TASK: run roapi-register-data
TASK->>RO: POST register sources (uk_cities, spacex_launches, blogs)
CI->>TASK: run cratedb-register-data
TASK->>CR: configure FDW and create foreign table (doc.uk_cities)
CI->>TASK: run cratedb-query-data
TASK->>CR: SELECT from doc.uk_cities -> return results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
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 |
| crash --hosts "http://cratedb:4200" <<SQL | ||
| SELECT * FROM doc.uk_cities; | ||
| SQL |
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.
Problem
At the last spot, when querying the foreign table in CrateDB, when CrateDB calls ROAPI, CrateDB currently responds with this error message:
PSQLException[ERROR: Invalid or Unsupported Configuration: could not find config namespace for key "application_name"]The reason is that ROAPI apparently does not accept this SQL statement yet.
SET application_name = 'PostgreSQL JDBC Driver'References
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.
We've reported our observations to the ROAPI authors, and also submitted a corresponding patch.
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.
ea3eb12 adds a custom OCI build, so we can consume the improvement early.
## Problem CrateDB's built-in PostgreSQL JDBC FDW connector emits this SQL statement, which currently fails on ROAPI: ```sql SET application_name = 'PostgreSQL JDBC Driver' ``` ## Solution Ignore all `SET key = 'value'` queries, or, to be more precise, all queries starting with `SET`. Is it too harsh? ## References - GH-415 - crate/cratedb-examples#1286 (review)
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 (1)
application/roapi/README.md (1)
32-35: Format URLs as proper Markdown links.The bare URLs work but violate Markdown best practices (markdownlint MD034). Consider formatting them as descriptive links for better readability.
Apply this diff:
## See also -- https://cratedb.com/docs/guide/feature/fdw/ -- https://cratedb.com/docs/crate/reference/en/latest/admin/fdw.html -- https://cratedb.com/blog/integrate-postgresql-with-cratedb-using-foreign-data-wrappers +- [CrateDB Guide: Foreign Data Wrappers](https://cratedb.com/docs/guide/feature/fdw/) +- [CrateDB Admin Documentation: FDW](https://cratedb.com/docs/crate/reference/en/latest/admin/fdw.html) +- [Blog: Integrate PostgreSQL with CrateDB using Foreign Data Wrappers](https://cratedb.com/blog/integrate-postgresql-with-cratedb-using-foreign-data-wrappers)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/application-roapi.yml(1 hunks)application/roapi/README.md(1 hunks)application/roapi/compose.yml(1 hunks)application/roapi/notes.md(1 hunks)application/roapi/roapi.Dockerfile(1 hunks)application/roapi/test.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- application/roapi/test.sh
- application/roapi/roapi.Dockerfile
- application/roapi/notes.md
- application/roapi/compose.yml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.
📚 Learning: 2025-07-23T22:00:51.593Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.
Applied to files:
application/roapi/README.md
📚 Learning: 2025-11-15T16:07:11.721Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1257
File: .github/workflows/lang-csharp-npgsql.yml:85-100
Timestamp: 2025-11-15T16:07:11.721Z
Learning: In the cratedb-examples repository CI workflows for .NET/Npgsql testing, conditional branches in test scripts (such as for "latest" versions) are intentionally kept even when not currently in the matrix, to allow easy toggling on/off by modifying the matrix values without changing the script logic.
Applied to files:
.github/workflows/application-roapi.yml
🪛 markdownlint-cli2 (0.18.1)
application/roapi/README.md
33-33: Bare URL used
(MD034, no-bare-urls)
34-34: Bare URL used
(MD034, no-bare-urls)
35-35: Bare URL used
(MD034, no-bare-urls)
⏰ 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: OS ubuntu-latest
🔇 Additional comments (5)
.github/workflows/application-roapi.yml (3)
1-19: LGTM!The workflow triggers and scheduling configuration are well-structured. The path filters ensure the workflow runs only when relevant files change, and the nightly schedule aligns with testing against CrateDB nightly builds.
21-24: LGTM!The concurrency configuration appropriately cancels in-progress jobs when new commits are pushed to the same branch, optimizing CI resource usage.
26-46: LGTM!The job configuration is straightforward and appropriate. The matrix strategy with a single OS provides easy extensibility for future multi-OS testing if needed. The TODO comment on line 44 indicates awareness of potential future refactoring to use the
ngrtest runner.application/roapi/README.md (2)
1-22: LGTM!The documentation clearly describes the purpose of the integration and provides a helpful overview of the components. The task definitions are well-documented, making it easy to understand the workflow.
24-30: LGTM!The Status section is refreshingly transparent about the current scope. The acknowledgment that this is an experimental integration focused on successful execution (exit code 0) rather than comprehensive validation aligns with the PR objectives.
Temporarily build ROAPI for CrateDB, until an obligatory improvement is included into an official release.
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
🧹 Nitpick comments (1)
application/roapi/compose.yml (1)
28-44: Add a healthcheck for better reliability.The ROAPI service lacks a healthcheck, which could lead to race conditions when dependent services attempt to connect before ROAPI is fully ready. Consider adding a healthcheck similar to the CrateDB service.
Apply this diff to add a healthcheck:
ports: - 8080:8080 - 5434:5432 - 32010:32010 + healthcheck: + test: ["CMD", "curl", "--fail", "http://localhost:8080/api/health"] + start_period: 3s + interval: 1.5s + retries: 30 + timeout: 30sNote: Verify that ROAPI exposes a health endpoint at
/api/healthor adjust the path accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/application-roapi.yml(1 hunks)application/roapi/README.md(1 hunks)application/roapi/compose.yml(1 hunks)application/roapi/notes.md(1 hunks)application/roapi/roapi.Dockerfile(1 hunks)application/roapi/test.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- application/roapi/notes.md
🚧 Files skipped from review as they are similar to previous changes (4)
- application/roapi/roapi.Dockerfile
- application/roapi/test.sh
- application/roapi/README.md
- .github/workflows/application-roapi.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.
📚 Learning: 2025-07-23T22:00:51.593Z
Learnt from: amotl
Repo: crate/cratedb-examples PR: 1038
File: application/open-webui/compose.yml:44-48
Timestamp: 2025-07-23T22:00:51.593Z
Learning: In the cratedb-examples repository, hard-coded credentials like "crate:crate" in Docker Compose files are acceptable for demonstration purposes to maintain simplicity and avoid unnecessary layers of indirection, even when flagged by security tools like Checkov.
Applied to files:
application/roapi/compose.yml
⏰ 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: OS ubuntu-latest
🔇 Additional comments (7)
application/roapi/compose.yml (7)
1-6: LGTM!The header provides clear references to the projects being integrated.
12-26: LGTM!The CrateDB service is properly configured for a demo environment with appropriate healthcheck settings.
49-54: LGTM!The crash utility is properly configured for on-demand interactive use.
59-76: LGTM with implicit dependency on Dockerfile.The data registration task correctly POSTs table definitions to ROAPI. Note that the
/test_data/URIs (lines 64, 67, 70) assume these files are included in the ROAPI container via the customroapi.Dockerfile.
94-114: LGTM!The CrateDB foreign data wrapper registration is correctly configured with proper dependencies on prior data registration steps.
116-128: LGTM!The query task has the correct dependency chain and properly validates the end-to-end integration.
136-142: LGTM with note on healthcheck consistency.The bundler correctly waits for dependencies, though the wait conditions differ (CrateDB uses
service_healthywhile ROAPI usesservice_started). This would be more consistent if ROAPI also had a healthcheck, as suggested in the earlier comment.
About
Explore the excellent ROAPI with CrateDB's built-in PostgreSQL JDBC foreign data wrapper.
Details
Targeting exit code zero, see CI run #20146253690.