-
Notifications
You must be signed in to change notification settings - Fork 13
Introduce docs from CPP Driver and adjust #357
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
base: master
Are you sure you want to change the base?
Conversation
61a7435
to
b266a03
Compare
87570af
to
10a19ad
Compare
* ScyllaDB CPP Driver lessons - [part 1](https://university.scylladb.com/courses/using-scylla-drivers/lessons/cpp-driver-part-1/) | ||
and [part 2](https://university.scylladb.com/courses/using-scylla-drivers/lessons/cpp-driver-part-2-prepared-statements/) at Scylla University. |
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.
Those lessons will need to be updated as well, right?
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.
Yes. But I'm not going to do this as part of this PR.
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.
Actually, how do they need to be updated? They mostly refer to the driver's API and basic working, so no special adjustments are needed IMO.
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 didn't look at the lessons, but don't they describe how to install the driver? This section will probably need to be updated (but of course not in this PR)
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.
don't they describe how to install the driver
No. They say:
To learn how to install it, see here.
So we need to update the link, that's it.
`${name}` is not a valid metavariable in the spec file - it was not substituted. Instead, it should be `%{name}`.
This is to align with the convention of using "contents" as the main table of contents file in documentation projects, as in the Rust Driver.
In its place, real documentation will be added later.
Let's name the Rust driver with its full name for clarity.
The `Doxyfile.in` file, which is used by the documentation generation workflow, has been added to the project straight from the cpp-driver. It's yet to be adjusted.
The `Doxyfile.in` file, which is used by the documentation generation workflow, has been adjusted for this project.
Ancient Cassandra versions are not supported by the driver, so it makes no sense to mention them in documentation and code.
docs/schema: fix typo client-side-timestamps: fix typo docs/schema_metadata: fix typo
We're not supporting DSE Cloud.
We don't support DSE.
When 7 years ago the default timestamp generator was changed from server-side to client-side, the documentation was improperly updated to reflect that change. This commit corrects the documentation.
...and warn about possible pitfalls of not setting a serial consistency before executing a Lightweight Transaction (LWT).
The implementation of CPP-Rust Driver has a limitation that callbacks must not block, otherwise the driver will panic. This commit documents this limitation in the documentation of futures, mentioning the notable exception of calling (otherwise blocking) future functions on futures that are guaranteed to be ready.
I believe the new name fits better.
Datatypes-related topics are now grouped under a single `data-types` directory.
The new name better reflects the semantics of this category.
10a19ad
to
d14d896
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.
Pull Request Overview
This PR introduces comprehensive documentation from the CPP Driver codebase and performs several cleanup tasks. The documentation is adapted for ScyllaDB, with outdated DataStax-specific references updated to reflect the current driver's capabilities. The PR also removes Kerberos/DSE support artifacts that are not relevant to the ScyllaDB CPP Driver.
- Imports comprehensive documentation structure covering driver usage, configuration, and architecture
- Updates all references from "Scylla" to "ScyllaDB" for consistency
- Removes deprecated DSE-specific features including Kerberos authentication and zlib dependencies
Reviewed Changes
Copilot reviewed 67 out of 70 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tests/src/integration/tests/*.cpp | Updates reference naming from "Scylla" to "ScyllaDB" in test comments |
tests/src/integration/integration.hpp | Updates comments and documentation references to use "ScyllaDB" |
tests/src/integration/embedded_ads.hpp/.cpp | Removes DSE-specific Kerberos authentication test infrastructure |
tests/CMakeLists.txt | Removes embedded ADS JAR configuration for DSE support |
src/driver_config.hpp | Removes Kerberos and zlib dependency definitions |
src/CMakeLists.txt | Removes Kerberos and zlib build configuration |
scylla-rust-wrapper/src/*.rs | Updates comments to use "ScyllaDB" instead of "Scylla" |
include/cassandra.h | Removes Cassandra version-specific documentation annotations |
examples/schema_meta/schema_meta.c | Removes Cassandra version-specific conditional code |
driver_config.hpp.in | Removes Kerberos and zlib configuration templates |
docs/source/topics/**/*.md | Adds comprehensive documentation covering driver usage, security, observability, and data types |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This commit removes the Windows build instructions from the documentation. The Windows build process is no longer verified and maintained, and therefore the instructions are removed to avoid confusion.
zlib is not a dependency of the CPP Rust driver, as the underlying Rust Driver has its own static dependencies.
As zlib is not used in the CPP-Rust Driver at all (it was used in CPP Driver for DataStax Cloud functionalities), this commit removes all references to zlib from the build system.
The driver does not support Kerberos authentication anymore, so the documentation has been updated to remove references to Kerberos and its installation instructions.
The embedded_ads code is related to Kerberos authentication to DSE clusters, which is no longer supported in the CPP-Rust Driver.
As Kerberos is not used in the CPP-Rust Driver at all (it was used in CPP Driver for custom authentication), this commit removes all references to Kerberos from the build system.
libuv is used by the CPP-Rust Driver only for tests and some of the examples, so let's state that clearly. Also, let's remove exact build instructions for libuv, as they are prone to change with new releases and it's better for the user to refer to the libuv documentation.
d14d896
to
5aeb383
Compare
@dgarcia360 Your review is needed, mainly for the last two commits. Feel free to push your commits with sphinx setup fixes on top of mine - I will amend mine with them once I'm back from vacation. |
@if [ ! -d "$(SOURCEDIR)" ]; then mkdir -p "$(SOURCEDIR)"; fi | ||
cp -RL source/* $(SOURCEDIR) | ||
# cd $(SOURCEDIR) && find . -name README.md -execdir mv '{}' index.md ';' |
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.
Lines 28 and 29 look ok.
We can remove lines 30–32 and set SOURCEDIR
to source
.
In this project, it does not appear necessary to copy the documentation to a separate _sources
folder for post-processing. That workflow is only required in projects where files need to be renamed at runtime, such as converting README.md
to index.md
.
docs/source/conf.py
Outdated
# Workaround to replace DataStax links | ||
replacements = [ | ||
{"http://datastax.github.io/cpp-driver/api/cassandra.h/": "https://cpp-rust-driver.docs.scylladb.com/" + smv_latest_version + "/api"}, | ||
{"http://datastax.github.io/cpp-driver": "https://cpp-rust-driver.docs.scylladb.com/" + smv_latest_version}, | ||
{"http://docs.datastax.com/en/developer/cpp-driver/latest": "https://cpp-rust-driver.docs.scylladb.com/" + smv_latest_version}, | ||
] | ||
sphinx.add_config_value('replacements', replacements, True) | ||
sphinx.connect('source-read', replace_relative_links) |
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.
There is only one reference to http://docs.datastax.com in this repository.
Lines 189-196 are only relevant for repositories forked from Datastax that contain unmaintained documentation.
I recommend deleting these lines and updating the affected links in the documentation to point to the correct Datastax resources.
|
||
**NOTE:** In this example keystore and truststore are identical. | ||
|
||
The following [guide](http://www.datastax.com/documentation/cassandra/2.1/cassandra/security/secureSSLClientToNode_t.html) has more information related to configuring TLS on ScyllaDB/Cassandra. |
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.
Is this link correct?
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.
Apologies for the delay. I left some comments.
Additionally, we might want to add doxygen/xml/
to .gitignore
.
Based on: #359
Fixes: #351
Hello, Docs!
Base: CPP Driver
CPP Driver setup
eval-rst
command to transform those READMEs to sphinx-readable resource tree, similarly to what Rust Driver does.conf.py
to transform Doxygen-generated files to sphinx.Transfer
Topics
directory).README.md
->index.md
, asindex
is not displayed in the URL, which is desired (so it'sfutures/
instead offutures/README.md
).feature/index.md
singleton pattern withfeature.md
to reduce needless directory nesting.Further adjustments
Cleanup
zlib
dependency, as it was only used for DSE support in CPP Driver, and never used in ScyllaDB CPP Driver.embedded_ads
from Integration Tests, as it's related to DSE support using Kerberos.libuv
is a non-mandatory dependency, as it is not used by the core ScyllaDB CPP Driver, only by tests and examples.ccm
in a VM.Restructuring
Substantive changes
as it differs in some ways compared to the original DataStax CPP Driver.
Attempted to set up
sphinx
(the last two commits)conf.py
to mimic CPP Driver's. This is to be reviewed and improved by @dgarcia360.Bonus
Found a typo in the RPM-generation script, which made it contain
${name}
string instead of the actual name of the package. Fixed it in the process.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.