Skip to content

Conversation

@amotl
Copy link
Member

@amotl amotl commented Dec 7, 2025

About

Explore the excellent ROAPI with CrateDB's built-in PostgreSQL JDBC foreign data wrapper.

Details

Targeting exit code zero, see CI run #20146253690.

@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025

Walkthrough

Adds 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

Cohort / File(s) Change Summary
CI / Workflow
/.github/workflows/application-roapi.yml
New GitHub Actions workflow "ROAPI": triggers on PRs and pushes to main (path filters), manual dispatch, nightly schedule; concurrency control; single test job that checks out sources and runs application/roapi/test.sh.
Documentation
application/roapi/README.md, application/roapi/notes.md
Adds README describing the ROAPI + CrateDB miniature stack, Compose tasks, and usage; notes.md adds backlog/runtime snippet and example runtime commands.
Compose / Orchestration
application/roapi/compose.yml
New Docker Compose config defining cratedb, roapi (built via roapi.Dockerfile), one‑off task services for registering/querying data, healthchecks, and depends_on conditions coordinating startup and task ordering.
Dockerfile / Build
application/roapi/roapi.Dockerfile
New multi‑stage Dockerfile: Rust tooling stage, builder stage pinned to ROAPI git rev 4d649d78 with --features database, and a slim final image copying the roapi binary and test data; exposes port 8080.
Tests / Scripts
application/roapi/test.sh
New shell script that brings up services with Docker Compose, runs ROAPI data registration task, mounts a CrateDB FDW table, and queries the mounted doc.uk_cities table in sequence.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to:
    • application/roapi/compose.yml: healthchecks, depends_on semantics, service ordering and one‑off task config
    • application/roapi/roapi.Dockerfile: pinned ROAPI revision, Rust toolchain and build flags, copying test data from external image
    • .github/workflows/application-roapi.yml: path filters, concurrency and schedule correctness
    • application/roapi/test.sh: error handling and alignment with compose service names

Suggested reviewers

  • kneth
  • surister

Poem

🐇 I stitched a stack of compose and cheer,
ROAPI and CrateDB hoppity near,
Tasks leap in order, tables take flight,
Tests hum softly through the night—
A rabbit’s build, all tidy and clear.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly matches the main change: adding a basic ROAPI example that demonstrates mounting CSV, JSON, and Parquet data sources.
Description check ✅ Passed The description is related to the changeset, explaining the purpose of exploring ROAPI with CrateDB's PostgreSQL JDBC foreign data wrapper, which aligns with the added example.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch roapi

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines +117 to +123
crash --hosts "http://cratedb:4200" <<SQL
SELECT * FROM doc.uk_cities;
SQL
Copy link
Member Author

@amotl amotl Dec 7, 2025

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

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.

houqp pushed a commit to roapi/roapi that referenced this pull request Dec 8, 2025
## 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)
@amotl amotl marked this pull request as ready for review December 11, 2025 20:22
coderabbitai[bot]

This comment was marked as resolved.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ea3eb12 and dd51371.

📒 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 ngr test 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.
Copy link

@coderabbitai coderabbitai bot left a 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: 30s

Note: Verify that ROAPI exposes a health endpoint at /api/health or adjust the path accordingly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd51371 and 4587104.

📒 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 custom roapi.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_healthy while ROAPI uses service_started). This would be more consistent if ROAPI also had a healthcheck, as suggested in the earlier comment.

@amotl amotl merged commit 8971dd5 into main Dec 16, 2025
2 checks passed
@amotl amotl deleted the roapi branch December 16, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants