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: introduce PrepareError #1193

Merged
merged 3 commits into from
Feb 3, 2025
Merged

Conversation

muzarski
Copy link
Contributor

@muzarski muzarski commented Feb 3, 2025

Ref: #519

As noticed by @wprzytula , after renaming QueryError to ExecutionError it is clear that Session::prepare should not return such error. This is why we introduce a PrepareError - an error that occurred during statement preparation. It's returned by Session::prepare method.

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.

This unnecessarily returned ExecutionError.
This is an error returned by `Session::prepare`
It was moved to PrepareError in previous commit.
@muzarski muzarski added the API-stability Part of the effort to stabilize the API label Feb 3, 2025
@muzarski muzarski added this to the 0.16.0 milestone Feb 3, 2025
@muzarski muzarski requested review from wprzytula and Lorak-mmk and removed request for wprzytula February 3, 2025 10:07
@github-actions github-actions bot added the semver-checks-breaking cargo-semver-checks reports that this PR introduces breaking API changes label Feb 3, 2025
Copy link

github-actions bot commented Feb 3, 2025

cargo semver-checks detected some API incompatibilities in this PR.
Checked commit: 773f0fd

See the following report for details:

cargo semver-checks output
./scripts/semver-checks.sh --baseline-rev 448502ca1db6228ab3d1d6108db78664e1dfddb6
+ cargo semver-checks -p scylla -p scylla-cql --baseline-rev 448502ca1db6228ab3d1d6108db78664e1dfddb6
     Cloning 448502ca1db6228ab3d1d6108db78664e1dfddb6
    Building scylla v0.15.0 (current)
       Built [  22.744s] (current)
     Parsing scylla v0.15.0 (current)
      Parsed [   0.051s] (current)
    Building scylla v0.15.0 (baseline)
       Built [  22.471s] (baseline)
     Parsing scylla v0.15.0 (baseline)
      Parsed [   0.049s] (baseline)
    Checking scylla v0.15.0 -> v0.15.0 (no change)
     Checked [   0.127s] 127 checks: 126 pass, 1 fail, 0 warn, 0 skip

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It 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.39.0/src/lints/enum_variant_missing.ron

Failed in:
  variant ProtocolError::PreparedStatementIdsMismatch, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-448502ca1db6228ab3d1d6108db78664e1dfddb6/65b2eff398a4ccb7c9c581eed694c6b5b77c88c7/scylla/src/errors.rs:195

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  46.413s] scylla
    Building scylla-cql v0.4.0 (current)
       Built [  11.410s] (current)
     Parsing scylla-cql v0.4.0 (current)
      Parsed [   0.035s] (current)
    Building scylla-cql v0.4.0 (baseline)
       Built [  11.087s] (baseline)
     Parsing scylla-cql v0.4.0 (baseline)
      Parsed [   0.034s] (baseline)
    Checking scylla-cql v0.4.0 -> v0.4.0 (no change)
     Checked [   0.127s] 127 checks: 127 pass, 0 skip
     Summary no semver update required
    Finished [  23.382s] scylla-cql
make: *** [Makefile:61: semver-rev] Error 1

@muzarski muzarski self-assigned this Feb 3, 2025
@wprzytula wprzytula merged commit 0a0480b into scylladb:main Feb 3, 2025
10 checks passed
@Lorak-mmk Lorak-mmk mentioned this pull request Mar 4, 2025
8 tasks
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