Skip to content

ci: revert running "Vs QuestDB master" against the nightly docker image (#160)#161

Open
ideoma wants to merge 1 commit into
mainfrom
revert-160-nightly-docker
Open

ci: revert running "Vs QuestDB master" against the nightly docker image (#160)#161
ideoma wants to merge 1 commit into
mainfrom
revert-160-nightly-docker

Conversation

@ideoma

@ideoma ideoma commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Reverts commit db63120 (the squash-merge of #160, "ci: run "Vs QuestDB master" against the nightly docker image").

This restores the previous from-source "Vs QuestDB master" job on hetzner-incus and removes the docker-based fixtures and helper changes introduced by #160:

  • ci/run_tests_pipeline.yaml — back to the from-source build + test.py run --repo / test_egress_failover.py run --repo.
  • system_test/fixture.py — removes QuestDbDockerFixture.
  • system_test/test.py / test_egress_failover.py — removes the --docker mode and DockerStandaloneInstance.
  • system_test/failover_clients/src/bin/exhaustion_client.rs — reverts the drain-after-kill change.

The diff is the exact inverse of db63120 (5 files, +100 / -540).

Summary by CodeRabbit

  • Tests

    • Updated system test infrastructure to validate against QuestDB master branch built from source instead of nightly Docker images.
    • Simplified failover exhaustion detection logic for more reliable test results.
    • Removed Docker-based testing mode from system test suite.
  • Chores

    • Updated CI pipeline to run integration tests on self-hosted infrastructure with enhanced environment setup.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR removes all Docker-backed QuestDB test infrastructure: QuestDbDockerFixture, DockerStandaloneInstance, --docker CLI flags, and related helpers are deleted from fixture.py, test.py, and test_egress_failover.py. The CI job TestVsQuestDBNightly is replaced by TestVsQuestDBMaster, which builds QuestDB from source on a self-hosted runner and runs tests via --repo. The exhaustion_client.rs post-kill batch check is simplified from a drain loop to a single call.

Changes

Docker Infrastructure Removal and Master Build Replacement

Layer / File(s) Summary
Remove Docker fixture and helpers
system_test/fixture.py
Deletes DOCKER_BIN, _docker, _unique_container_name, QuestDbDockerFixture, and the tempfile import, leaving only local-process and TLS proxy fixtures.
Remove Docker mode from test runners
system_test/test.py, system_test/test_egress_failover.py
Removes --docker CLI arguments, QuestDbDockerFixture import, iter_versions Docker branch, _new_fixture helper, and DockerStandaloneInstance class. Replaces all Docker-conditional fixture construction with direct QuestDbFixture and StandaloneInstance calls across all test suites.
Simplify exhaustion client post-kill check
system_test/failover_clients/src/bin/exhaustion_client.rs
Replaces the buffered-batch drain loop with a single cursor.next_batch() call after harness kill: Ok exits as failure, Err captures and returns exhausted_code.
Add TestVsQuestDBMaster CI job
ci/run_tests_pipeline.yaml
Replaces TestVsQuestDBNightly with TestVsQuestDBMaster on hetzner-incus self-hosted runners. Adds setup steps for OS packages, JDK 17/25 validation, Rust toolchain, Maven compilation of QuestDB from source, and test execution with --repo ./questdb. Updates failure artifact path to build/questdb/repo/data/log and artifact name to qdb-log-vs-master-*.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • questdb/c-questdb-client#151: Adds and wires the shared templates/compile.yaml and templates/clone_questdb.yaml templates that the new TestVsQuestDBMaster job invokes directly.
  • questdb/c-questdb-client#160: Modifies the same system test files (fixture.py, test.py, test_egress_failover.py, exhaustion_client.rs) with overlapping Docker-mode and post-kill exhaustion-batch handling changes.

Suggested reviewers

  • jerrinot

Poem

🐇 Hop hop, no more Docker to pull,
We build from master, the source in full!
The nightly image? Gone with a bound,
On hetzner-incus, fresh builds are found.
One batch call suffices, no loop to drain—
The rabbit runs tests on the master's lane! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reverting commit db63120 to restore from-source QuestDB master testing instead of nightly Docker image testing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch revert-160-nightly-docker

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
ci/run_tests_pipeline.yaml (1)

243-259: ⚡ Quick win

Archive the egress failover server logs too.

If test_egress_failover.py fails, the useful QuestDB logs are under build/questdb/server1/log and build/questdb/server2/log, but this artifact only captures build/questdb/repo/data/log. Please include the standalone server log directories so failures in the new egress step are diagnosable from CI artifacts.

Proposed artifact path adjustment
-              rootFolderOrFile: $(Build.SourcesDirectory)/build/questdb/repo/data/log
+              rootFolderOrFile: $(Build.SourcesDirectory)/build/questdb
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/run_tests_pipeline.yaml` around lines 243 - 259, The ArchiveFiles@2 task
is only capturing logs from the main QuestDB server directory
(build/questdb/repo/data/log), but when test_egress_failover.py fails, the
diagnostic logs are in the failover server directories
(build/questdb/server1/log and build/questdb/server2/log). Modify the
rootFolderOrFile input in the ArchiveFiles@2 task to include all three log
directories, or create additional ArchiveFiles@2 and PublishBuildArtifacts@1
tasks to separately archive and publish the failover server logs (server1 and
server2) so that egress failover test failures can be diagnosed from CI
artifacts.
system_test/failover_clients/src/bin/exhaustion_client.rs (1)

46-151: ⚡ Quick win

Reminder: Run cargo fmt and cargo clippy before committing.

As per coding guidelines, please run the following commands before committing Rust changes:

cargo fmt --manifest-path system_test/failover_clients/Cargo.toml
cargo clippy --manifest-path system_test/failover_clients/Cargo.toml --tests
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@system_test/failover_clients/src/bin/exhaustion_client.rs` around lines 46 -
151, The code in the main function has not been formatted or linted according to
Rust coding guidelines. Before committing this code, run the Rust formatter and
linter tools on the failover_clients project to ensure proper code style and
catch any potential issues. Use cargo fmt to format the code and cargo clippy to
identify and flag any non-idiomatic patterns or common mistakes in the
exhaustion_client.rs file and related code.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ci/run_tests_pipeline.yaml`:
- Around line 243-259: The ArchiveFiles@2 task is only capturing logs from the
main QuestDB server directory (build/questdb/repo/data/log), but when
test_egress_failover.py fails, the diagnostic logs are in the failover server
directories (build/questdb/server1/log and build/questdb/server2/log). Modify
the rootFolderOrFile input in the ArchiveFiles@2 task to include all three log
directories, or create additional ArchiveFiles@2 and PublishBuildArtifacts@1
tasks to separately archive and publish the failover server logs (server1 and
server2) so that egress failover test failures can be diagnosed from CI
artifacts.

In `@system_test/failover_clients/src/bin/exhaustion_client.rs`:
- Around line 46-151: The code in the main function has not been formatted or
linted according to Rust coding guidelines. Before committing this code, run the
Rust formatter and linter tools on the failover_clients project to ensure proper
code style and catch any potential issues. Use cargo fmt to format the code and
cargo clippy to identify and flag any non-idiomatic patterns or common mistakes
in the exhaustion_client.rs file and related code.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc53c74f-68e7-45da-a5d6-7ef367760e25

📥 Commits

Reviewing files that changed from the base of the PR and between db63120 and ed1a1f7.

📒 Files selected for processing (5)
  • ci/run_tests_pipeline.yaml
  • system_test/failover_clients/src/bin/exhaustion_client.rs
  • system_test/fixture.py
  • system_test/test.py
  • system_test/test_egress_failover.py
💤 Files with no reviewable changes (1)
  • system_test/fixture.py

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.

1 participant