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

refactor(0.7.x): 0.7.5 with exposed non-retryable s3_fetchers #98

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

khorolets
Copy link
Member

This PR should address #97 and bump the version of the crate to 0.7.5

I had to extract the code from the infinity loops of fetch_block_or_retry and a similar function for the shards.

I made two new functions: fetch_block and fetch_shard, and made them public.

@khorolets
Copy link
Member Author

cc @kobayurii

@kobayurii
Copy link
Member

LGTM! Thank you a lot!


[dependencies]
anyhow = "1.0.51"
aws-config = { version = "1.0.0", features = ["behavior-version-latest"] }
aws-types = "1.0.0"
aws-credential-types = "1.0.0"
aws-sdk-s3 = "0.39.1"
aws-smithy-types = "1.0.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to move it from dev-dependencies to a general one since I introduced a variant with aws-smithy-types error to LakeError regarding the byte-stream

.await?
.body
.collect()
.await?
Copy link
Member Author

Choose a reason for hiding this comment

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

This one will convert into new LakeError::AwsSmithyError

src/types.rs Outdated
IntConversionError {
#[from]
error: std::num::TryFromIntError,
},
#[error("AWS Smithy byte_stream error: {error}")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[error("AWS Smithy byte_stream error: {error}")]
#[error("AWS Smithy byte_stream error: {error:?}")]

Can we print the debug message, the normal display message doesn't provide much information. Same for AwsError too please 🙏🏼

@khorolets khorolets merged commit 554c493 into 0.7.x Dec 19, 2023
5 checks passed
@khorolets khorolets deleted the refactor/expose-fetchers branch December 19, 2023 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants