Examples fix table names #4
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nsipplswezey
force-pushed
the
examples-fix-table-names
branch
6 times, most recently
from
c021936 to
c8abb78
Compare
In January the tests started failing on Cassandra `4.1`, so we pinned the cassandra version to `4.0.7`, which didn't have these problems. `4.0.7` is the version that we've been using in the CI since then. In the meantime there were new releases of `Cassandra`, at the moment the latest is `4.1.3`. Let's try to go back to the latest version, maybe the problems fixed themselves during the last nine months. Refs: scylladb#620 Refs: scylladb#629 Fixes: scylladb#633 Signed-off-by: Jan Ciolek <[email protected]>
The recent fix that changed the `types::write_short` function to use u16 instead of i16 broke one of our benchmarks - it tried to call that function with a negative number. The benchmarks aren't compiled in our CI, so it didn't catch it. Fix the issue by using `u16::MAX` instead of `-1`.
Our previous combination of the flags didn't build or check the benchmarks at all. In order to prevent this from happening in the future, adjust the `cargo check`, `cargo clippy` and `cargo build` invocations to use the `--all-targets` flag.
…-targets-in-ci benchmarks/CI: make benchmarks compile and build them in CI
It will be used by min_rust workflow to build and check with our MSRV to prevent errors from packages updating their MSRV.
Commit renamed Cargo.lock file
cassandra/docker-compose: go back to using the latest cassandra version
Adds support of 'time' and 'chrono' types for CQL queries. Changelist: - Rework scylla-cql 'Time', 'Date' and 'Timestamp' types to align with CQL data representation; - Add 'Cql'- prefix to builtin date and time types; - Implement conversion traits between chrono, time and Cql- types; - Implement CQL 'Value' trait for 'chrono' and 'time' types; - Implement 'FromCqlVal' trait for 'chrono' and 'time' types. - Encapsulate CQL types in a wrapper Closes scylladb#745
…neric_args A new clippy lint started complaining about the `check_ref_tuple` test: its inner `assert_has_batch_values`, supposedly, needlessly borrows its argument. As it looks like one of the points of the test is to verify that batch values taken by reference still implement batch values, the clippy lint seems incorrect. Silence the lint by explicitly specifying that the type of the generic argument for `assert_has_batch_values` is a reference.
value_tests: fix clippy's new complaint about needless_borrows_for_generic_args
Introduce the `read_value` function which is able to read a [value], as specified in the CQL protocol. It will be used in the next commit, in order to make the interface of the SerializedValue iterators more correct.
Currently, the SerializedValues' `iter()` method treats both null and unset values as None, and `iter_name_value_pairs()` just assumes that values are never null/unset and panics if they are. Make the interface more correct by adjusting both methods to return RawValue. The iterators will be used in the next commit to implement the fallback that allows to implement `SerializeRow`/`SerializeCql` via legacy `ValueList`/`Value` traits.
There is no way to filter out Execute messages like we can with Query messages, so if there are some sent by the driver itself (which will start happening soon) we can't reliably perform tests using proxy. This commit implements a heuristic way to filter out control connection messages. The idea is that only control connection registers to CQL Events, so we can skip connections that issued any registration message.
Some examples in scylla-proxy use tokio::signal, but this feature is not enabled in dependencies, and this prevents running cargo test in scylla-proxy directory. For some reason cargo test works when run in workspace, I'm not sure why that is the case.
…conn scylla-proxy: Add option to filter out control connection messages
… interfaces Currently, `SerializeRow` and `SerializeCql` traits are just given a mutable reference to a Vec<u8> and asked to append their CQL representation to the end. While simple, there are some issues with the interface: - The serialize method has access to the serialized representation of the values that were appended before it. It's not necessary for a correct implementation to have access to it. - Implementors technically can append any byte sequence to the end, but actually are expected to produce a CQL [value] containing the serialized value. While the `SerializeRow` and `SerializeCql` traits are not generally meant to be manually implemented by the users, we can make the interface easier to use and harder to misuse by making it append-only, restricting what the users are allowed to append and requiring the users to append anything by using a dash of type-level magic. Introduce `RowWriter` and `CellWriter` traits which satisfy the above wishes and constraints, and pass them instead of Vec<u8> in `SerializeRow` and `SerializeCql`. The new traits have two implementations - a Vec<u8> backed one that actually appends the bytes given to it, and a usize-backed one which just measures the length of the output without writing anything. Passing the latter before doing the actual serialization will allow to preallocate the right amount of bytes and then serialize without reallocations. It should be measured whether the reallocation cost always outweighs the calculation cost before implementing this optimization.
types: serialize: constrain the new serialization traits to make them easier and safer to use
Error needs to be dynamic, so that users can use their own types, but at the same time allow it to be stored in concrete type (like `QueryError`). `dyn Error` supports downcasting, so it can be used instead of `Any`.
Implementations such as `impl From<SerializationError> for QueryError` don't seem like a good idea when SerializationError is just an alias for `Arc<dyn Error + Send + Sync>`. Make SerializationError a newtype to avoid such general implementations.
…wtype Make SerializationError a newtype
Some conversions between time-related types return `ValueTooBig` in case when one representation doesn't fit into the other. This is a misinterpretation of the `ValueTooBig`'s semantics: it is supposed to be returned when the value's size in bytes is longer than the maximum size of `i32::MAX`, not when the value overflows the allowed range (e.g. doesn't fit in a 64bit number). Introduce `ValueOverflow` error in order to differentiate the cases when a value is "too big" and use it in conversions between time-related types. Adjust relevant `Value` implementations which use the conversions to map the `ValueOverflow` to `ValueTooBig`. This is done in order not to introduce breaking changes to the soon-to-be-legacy interface.
Adjust the existing tests for the `Value` trait to also do serialization via `SerializeCql` and compare results produced by both traits. The tests pass at the moment because the new trait is implemented using the old one. In the commits that follow, we will introduce proper implementations of the new trait for the existing types. In nearly all cases we will be interested in preserving compatibility, so the exising tests will be very helpful.
Scylla's Sphinx theme doesn't seem to support tables.
…erializeCql`'s Fixes scylladb#897
In some places, we have the following expression:
x.iter().map(|(k, v)| (k, v))
Although the .map call looks like a no-op, it changes the type of the
argument from `&(K, V)` to `(&K, &V)`. This happens due to the so-called
"reference matching ergonomics" feature of Rust which allows binding a
reference to a tuple to the `(k, v)` expression and allows referring to
the tuple's constituents by reference (`k: &K`, `v: &V`).
The issue is already known and reported and will most likely be fixed
in a future release, but for the time being let's reformulate the code
to avoid the lint like this:
x.iter().map(|p| (&p.0, &p.1))
Clippy suggests that .first() is more readable than .get(0).
scylla-cql, examples: fix clippy 1.75 issues
scylla-cql: Use `#[cfg(feature = "time")]` for `time` related `impl SerializeCql`'s
Add `skip` attribute to serialization proc macros
This will be required to move documentation of serialization macros into scylla crate. This in turn is required to workaround the issue that documentation for those macros is not rendered on docs.rs correctly.
When both original item and re-export are documented, documentations are merged. Before this commit, both original item and re-export were documented - identically or at least very similarly. This caused docs in scylla-cql crate to be copy-pasted twice. Replace docs with stubs pointing to the scylla crate to avoid it and point users to proper documentation.
This module is now so small it doesn't make much sens to keep it in separate file.
Documentation was moved from scylla-cql to scylla, which made some paths used in those docs incorrect. This commit updates paths to correct ones.
Rustdocs merges documentation of re-exports with documentation of original item. Because of this there is a need for separator, so that stubs from scylla-macros crate are not mixed up with docs present in scylla crate.
Without those lines, whole doc for an item is merged and shown in the summary, which makes it hard to read.
This is to avoid having to use build_book.py script. 1. Preprocessor is faster and doesn't use disk, as all communication is trough stdio. 2. It doesn't need to be executed explicitly. You can just use `mdbook build docs` and it will create correct documentation. 3. New script is shorter and simpler than old. Script is based on example from https://rust-lang.github.io/mdBook/for_developers/preprocessors.html and uses a function from build_book.py In book.toml the command used is a simple bash script instead of just `python ./sphinx_preprocessor.py` because the command is run in the directory the user executed it in, not the directory that book.toml is in (which is imho a poor design in this case), so in order to be able to run the command both from main dir and from docs dir, such a script locating the preprocessor is necessary.
This script is no longer necessary and just double the work
Docs: Add a book preprocessor
Move derive macros docs to scylla crate
ks is changed from ks to examples_ks table names are changed from t to the example file name Examples share an explicitly named keyspace Examples now have unique table names preventing errors that occur from two examples sharing the same table
piodul
force-pushed
the
examples-fix-table-names
branch
from
c8abb78 to
86e42f6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kick off CI with examples on local fork
Pre-review checklist
./docs/source/.Fixes:annotations to PR description.