-
Notifications
You must be signed in to change notification settings - Fork 13
Run examples in CI #336
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
Run examples in CI #336
Conversation
8e5c8ca
to
291be5c
Compare
3df182f
to
ea17b61
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.
Commit: examples: add drop_examples_keyspace example
The other alternative is to make examples also work with existing keyspaces (`CREATE KEYSPACE / TABLE IF NOT EXISTS). Imo it would be cleaner, not requiring this additional weird target.
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.
They would also have to drop the keyspace beforehand, because they often write the same data to the same keyspace/table. I'm still not convinced it's worth modyfing the examples.
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.
I don't really understand why use docker here. You have already existing workflows in CI that use CCM. You could do the same instead of introducing another method.
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.
Which workflows in CI use CCM? Well, I agree that integration tests use CCM. However, they are calling it through the bridge layer, from C++. Here I'd have to use it straight from the CLI. Why is it better than a ready docker-compose taken from Rust Driver?
ea17b61
to
aac3ca5
Compare
Rebased on master. |
aac3ca5
to
3378226
Compare
Rebased on master, dropping the commit that adjusts the examples from VARCHAR to TEXT. |
The example will be used by the example runner in Makefile to drop the `examples` keyspace between runs of consecutive examples. This is because the examples often create the `example` keyspace, don't drop it, and at the same time expect that the keyspace does not exist when they run. To avoid this, we add a new example that drops the `examples` keyspace, and the example runner will run it before running the next example. Alternatively, we could use cqlsh, but this would require adding it as a new dependency, which is not desirable.
The simplest way to run examples in CI is to use docker-compose. The docker-compose file, adapted from the Rust Driver, is configured to run a single Scylla node.
The driver does not support DSE, so let's remove the examples.
Some examples, e.g. `callbacks`, require libuv, so we enable it when building examples.
`cpp_integration_testing` is now required to build examples, because some of them use functions that are only available when `cpp_integration_testing` config option is enabled (because they are only implemented as stubs for testing purposes).
3378226
to
4d5a7d9
Compare
Rebased on master. |
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.
Pull Request Overview
This PR adds CI support for building and running examples against a ScyllaDB Docker container. The implementation includes comprehensive stub additions for unimplemented functions required by examples, infrastructure for running examples in CI, and cleanup of DSE-specific examples that are not applicable to ScyllaDB.
- Adds function stubs for authenticator, host listener, and version types to enable example compilation
- Introduces Docker-based ScyllaDB test infrastructure with automated example execution
- Removes DSE-specific examples (geotypes, date_range, gssapi, plaintext, proxy_execution) that don't apply to ScyllaDB
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/examples_cluster/docker-compose.yml | Adds ScyllaDB Docker container configuration for example testing |
scylla-rust-wrapper/src/lib.rs | Imports new type modules for authenticator, host listener, and version types |
scylla-rust-wrapper/src/integration_testing.rs | Expands stub functions and exports for examples compilation |
scylla-rust-wrapper/build.rs | Adds bindgen preparation for new type modules |
scylla-rust-wrapper/CMakeLists.txt | Enables integration testing flags for examples builds |
Makefile | Adds targets for building and running examples with ScyllaDB |
CMakeLists.txt | Links libuv for examples that require it |
.github/workflows/build-lint-and-test.yml | Integrates example execution into CI pipeline |
examples/drop_examples_keyspace/* | Refactors example to use standard Cassandra API instead of DSE |
examples/dse/* | Removes DSE-specific examples not applicable to ScyllaDB |
Makefile have been updated to support building examples.
Makefile now includes a variable `SCYLLA_EXAMPLES_TO_RUN`, which lists the examples that are expected to run successfully. In a next commit, this variable will be used to run the examples in another Make target.
The `run-examples-scylla` target is added to the Makefile to run examples. The examples are built and run against a ScyllaDB instance running in a Docker container. The `SCYLLA_EXAMPLES_TO_RUN` variable is used to specify which examples to build and run. As some examples create the `examples` keyspace and some assume it is not present, a special example `drop_examples_keyspace` is run to drop the `examples` keyspace before running any example. This lets us avoid adding a dependency on `cqlsh` to the system.
This makes examples run in the CI workflow, ensuring that they are tested and functional.
4d5a7d9
to
3453eb4
Compare
|
||
if(CASS_BUILD_INTEGRATION_TESTS) | ||
if(CASS_BUILD_INTEGRATION_TESTS OR CASS_BUILD_EXAMPLES) | ||
set(CMAKE_Rust_FLAGS "${CMAKE_Rust_FLAGS} --cfg cpp_integration_testing") | ||
endif() | ||
|
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 need to make sure this flag is not set in published binaries - we don't want stubs there.
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.
I've read the packaging logic carefully and my conclusion is that the used build command is correct wrt not including stubs in the generated package.
For redhat derivates, this line is important (dist/redhat/scylla-cpp-rust-driver.spec
):
%cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_RustFLAGS="-Cforce-frame-pointers=yes --cap-lints=warn"
For debian, it's similar (dist/debian/debian/rules
):
dh_auto_configure -- -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_RustFLAGS="--cap-lints warn -C linker=$(DEB_HOST_GNU_TYPE)-gcc -C link-arg=-Wl,-Bsymbolic-functions -C link-arg=-Wl,-z,relro"
I'm suspicious about the --cap-lints warn
part. Why would anyone want to include this? Essentially, this converts all errors to warnings, effectively making the compilation process pass even if it would fail otherwise.
I've talked to ChatGPT on this topic.
What's done
Fixes: #305
Pre-review checklist
[ ] I have implemented Rust unit tests for the features/changes introduced.[ ] I have enabled appropriate tests inMakefile
in{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER
.Fixes:
annotations to PR description.