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

iterator: delete TypedRowLendingStream as unusable #1122

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

wprzytula
Copy link
Collaborator

We failed to find a way to make TypedRowLendingStream usable. The borrow checker keeps forbidding making more than one call to next() method.

In the close future, we'll investigate alternatives to bring borrowed types deserialization capabilities to QueryPager.

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.

We failed to find a way to make TypedRowLendingStream usable. The borrow
checker keeps forbidding making more than one call to `next()` method.

In the close future, we'll investigate alternatives to bring borrowed
types deserialization capabilities to QueryPager.
@wprzytula wprzytula self-assigned this Nov 12, 2024
@wprzytula wprzytula added this to the 0.15.0 milestone Nov 12, 2024
@github-actions github-actions bot added the semver-checks-breaking cargo-semver-checks reports that this PR introduces breaking API changes label Nov 12, 2024
Copy link

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

See the following report for details:

cargo semver-checks output
./scripts/semver-checks.sh --baseline-rev 814de55d710391cfe6e649b66c16e2b92c4a2a7f
+ cargo semver-checks -p scylla -p scylla-cql --baseline-rev 814de55d710391cfe6e649b66c16e2b92c4a2a7f
     Cloning 814de55d710391cfe6e649b66c16e2b92c4a2a7f
     Parsing scylla v0.14.0 (current)
      Parsed [  28.169s] (current)
     Parsing scylla v0.14.0 (baseline)
      Parsed [  22.764s] (baseline)
    Checking scylla v0.14.0 -> v0.14.0 (no change)
     Checked [   0.118s] 94 checks: 92 pass, 2 fail, 0 warn, 0 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn 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.36.0/src/lints/inherent_method_missing.ron

Failed in:
  QueryPager::rows_lending_stream, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-814de55d710391cfe6e649b66c16e2b92c4a2a7f/53995b156e5bf29b0d33e08f437c41da85f6e2a7/scylla/src/transport/iterator.rs:652

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct 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.36.0/src/lints/struct_missing.ron

Failed in:
  struct scylla::transport::iterator::TypedRowLendingStream, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-814de55d710391cfe6e649b66c16e2b92c4a2a7f/53995b156e5bf29b0d33e08f437c41da85f6e2a7/scylla/src/transport/iterator.rs:989

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  51.107s] scylla
     Parsing scylla-cql v0.3.0 (current)
      Parsed [  11.299s] (current)
     Parsing scylla-cql v0.3.0 (baseline)
      Parsed [  11.284s] (baseline)
    Checking scylla-cql v0.3.0 -> v0.3.0 (no change)
     Checked [   0.106s] 94 checks: 94 pass, 0 skip
     Summary no semver update required
    Finished [  22.738s] scylla-cql
make: *** [Makefile:61: semver-rev] Error 1

Copy link
Collaborator

@Lorak-mmk Lorak-mmk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its surprising how bad is the diff generated by GitHub

@wprzytula wprzytula merged commit 27ce6b5 into scylladb:main Nov 12, 2024
11 checks passed
@wprzytula wprzytula deleted the delete-lending-stream branch November 12, 2024 20:25
@wprzytula wprzytula mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/deserialization 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.

2 participants