Skip to content

adding retryable to scan #456

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.

Sign up for GitHub

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

Merged
merged 12 commits into from
Aug 22, 2024
Merged

adding retryable to scan #456

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6e52912
adding retryable to scan
limbooverlambda Jun 17, 2024
da1785e
PR feedback along with a unit test
limbooverlambda Jun 25, 2024
dcb6b89
make check fixes
limbooverlambda Jun 26, 2024
181cde1
address PR feedback
limbooverlambda Jul 8, 2024
3ee7eff
removing the lambda to simplify the retry logic
limbooverlambda Aug 5, 2024
70a3771
adding protobuf-code feature to cargo.toml
limbooverlambda Aug 8, 2024
8bdd1d5
added clippy fix for the formatting issues with the latest stable
limbooverlambda Aug 8, 2024
6b40932
moving the region and store lookup inside the loop and removing the c…
limbooverlambda Aug 10, 2024
a7b4b74
added a fix for the bug with the range
limbooverlambda Aug 12, 2024
8c6db45
merging with master
limbooverlambda Aug 12, 2024
0a816f6
removing the range attribute from ScanInnerArgs and replacing it with…
limbooverlambda Aug 19, 2024
a801c76
Merge branch 'master' into limbooverlambda/fixsignature
pingyu Aug 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ generate:
check: generate
cargo check --all --all-targets --features "${ALL_FEATURES}"
cargo fmt -- --check
cargo clippy --all-targets --features "${ALL_FEATURES}" -- -D clippy::all
cargo clippy --fix --allow-dirty --all-targets --features "${ALL_FEATURES}" -- -D clippy::all
Copy link
Collaborator

@pingyu pingyu Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's not a good idea to add --fix --allow-dirty, as the fix will be done in CI only and not applied to code base. Besides, some automatic change may not be correct, we need to have a check.

I will open another PR in these few days to fix the errors introduced by the latest toolchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @pingyu . It seems like the toolchain upgrade started flagging the autogenerated docs. --fix --allow-dirty is a workaround to make the build go through. I did't want to pin the toolchain to a certain version, so this was a quick way to bypass the clippy errors. I agree with you on not using --fix --allow-dirty. Do you have any suggestion on how I can get this PR merged? Looks like all the tests are passing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not a good idea to add --fix --allow-dirty, as the fix will be done in CI only and not applied to code base. Besides, some automatic change may not be correct, we need to have a check.

I will open another PR in these few days to fix the errors introduced by the latest toolchain.

The check errors for latest toolchain will be fixed by #460.


unit-test: generate
cargo nextest run --all --no-default-features
Expand Down
Loading