Skip to content
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

errors: adjust ShardingError #1181

Merged
merged 5 commits into from
Jan 28, 2025
Merged

Conversation

muzarski
Copy link
Contributor

Ref: #1170

The only breaking change in this PR is unpubbing ShardingError. Remaining commits adjust the internal usages of this error - for example logging in case parsing of sharding info fails.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [ ] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • [ ] I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

wprzytula and others added 3 commits January 27, 2025 18:05
This makes the code more rusty and leverages the type system. Also, it
extracts the constants for shared use in the next commit.
I added a new error variant to distinguish between a scenario where we (most likely)
connect to Cassandra cluster (all params missing) and where Scylla has a bug
(some params are present, some not).
@muzarski muzarski self-assigned this Jan 27, 2025
@muzarski muzarski added this to the 0.16.0 milestone Jan 27, 2025
@muzarski muzarski added the API-stability Part of the effort to stabilize the API label Jan 27, 2025
@github-actions github-actions bot added the semver-checks-breaking cargo-semver-checks reports that this PR introduces breaking API changes label Jan 27, 2025
Copy link

github-actions bot commented Jan 27, 2025

cargo semver-checks detected some API incompatibilities in this PR.
Checked commit: 44cae1f

See the following report for details:

cargo semver-checks output
./scripts/semver-checks.sh --baseline-rev bb824b61cefb8426cc00aef563ce27882cf0aa7c
+ cargo semver-checks -p scylla -p scylla-cql --baseline-rev bb824b61cefb8426cc00aef563ce27882cf0aa7c
     Cloning bb824b61cefb8426cc00aef563ce27882cf0aa7c
    Building scylla v0.15.0 (current)
       Built [  24.106s] (current)
     Parsing scylla v0.15.0 (current)
      Parsed [   0.052s] (current)
    Building scylla v0.15.0 (baseline)
       Built [  22.480s] (baseline)
     Parsing scylla v0.15.0 (baseline)
      Parsed [   0.051s] (baseline)
    Checking scylla v0.15.0 -> v0.15.0 (no change)
     Checked [   0.113s] 107 checks: 106 pass, 1 fail, 0 warn, 0 skip

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/enum_missing.ron

Failed in:
  enum scylla::routing::ShardingError, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-bb824b61cefb8426cc00aef563ce27882cf0aa7c/8e88ff90ea17547a103649e6571db48be24ce469/scylla/src/routing/sharding.rs:99

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  48.008s] scylla
    Building scylla-cql v0.4.0 (current)
       Built [  10.816s] (current)
     Parsing scylla-cql v0.4.0 (current)
      Parsed [   0.035s] (current)
    Building scylla-cql v0.4.0 (baseline)
       Built [  10.987s] (baseline)
     Parsing scylla-cql v0.4.0 (baseline)
      Parsed [   0.035s] (baseline)
    Checking scylla-cql v0.4.0 -> v0.4.0 (no change)
     Checked [   0.110s] 107 checks: 107 pass, 0 skip
     Summary no semver update required
    Finished [  22.503s] scylla-cql
make: *** [Makefile:61: semver-rev] Error 1

scylla/src/network/connection.rs Outdated Show resolved Hide resolved
Lorak-mmk
Lorak-mmk previously approved these changes Jan 28, 2025
I chose INFO log level in case ALL parameters are missing (no sharding info provided).
ERROR otherwise - in case there was some failure in parsing the sharding info.
This error type is used only internally. The `ShardInfo` struct is
pub(crate) as well.
@wprzytula wprzytula merged commit b6a3e01 into scylladb:main Jan 28, 2025
9 checks passed
@muzarski muzarski deleted the sharding-error branch January 28, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-stability Part of the effort to stabilize the API semver-checks-breaking cargo-semver-checks reports that this PR introduces breaking API changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants