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

[deps] bump prost & tonic #1616

Merged
merged 2 commits into from
Aug 27, 2021
Merged

Conversation

PsiACE
Copy link
Member

@PsiACE PsiACE commented Aug 26, 2021

Signed-off-by: Chojan Shang psiace@outlook.com

I hereby agree to the terms of the CLA available at: https://datafuse.rs/policies/cla/

Summary

Changelog

  • Improvement
  • Build/Testing/CI

Related Issues

Fixes #1090 #1027

Test Plan

Unit Tests

Stateless Tests

Signed-off-by: Chojan Shang <psiace@outlook.com>
@databend-bot databend-bot added pr-build this PR changes build/testing/ci steps pr-improvement labels Aug 26, 2021
@databend-bot
Copy link
Member

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

@databend-bot
Copy link
Member

Hello @PsiACE, 🎉 Thank you for opening the pull request! 🎉
Your pull request state is not in Draft, please add Reviewers or Re-request review again!
FuseQuery: @BohuTANG @sundy-li @zhang2014
FuseStore: @drmingdrmer @dantengsky
Or visit datafuse roadmap for some clues.

@PsiACE
Copy link
Member Author

PsiACE commented Aug 26, 2021

This PR is ready. Of course we can also wait for arrow2.

@@ -15,8 +15,8 @@ simd = ["arrow/simd"]
# Workspace dependencies

# Github dependencies
arrow = { package = "arrow2", git="https://github.com/datafuse-extras/arrow2", rev = "43c101f" }
arrow-flight = { git="https://github.com/datafuse-extras/arrow2", rev = "43c101f" }
arrow = { package = "arrow2", git="https://github.com/psiace/arrow2", rev = "5187d3d" }
Copy link
Member

Choose a reason for hiding this comment

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

😎, why not using the datafuse-extra branch here?
What's the difference between them?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems that I still need to solve the problem of arrow2 integration test, although it can already be used in our project :(

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, how about wait the arrow2 merged or patch it to the datafuse-extra/arrow2?

Copy link
Member Author

Choose a reason for hiding this comment

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

Later, maybe 20 minutes

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@drmingdrmer drmingdrmer left a comment

Choose a reason for hiding this comment

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

Looks good!.
But these deps do not affect a lot of the codes I am familiar with.
So, leave the final approve to somebody else.

metadata.insert_bin(AUTH_TOKEN_KEY, MetadataValue::from_bytes(&token));
Ok(req)
})
FlightServiceClient::with_interceptor(channel, AuthInterceptor { token })
Copy link
Member

Choose a reason for hiding this comment

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

It's nice~

Signed-off-by: Chojan Shang <psiace@outlook.com>
@PsiACE PsiACE requested a review from BohuTANG August 27, 2021 00:12
@PsiACE
Copy link
Member Author

PsiACE commented Aug 27, 2021

Once merged, we can enable Pull Request Security Audit again

Copy link
Member

@BohuTANG BohuTANG left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you @PsiACE

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2021

Codecov Report

Merging #1616 (bbf0e41) into master (47510e7) will increase coverage by 0%.
The diff coverage is 66%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1616   +/-   ##
======================================
  Coverage      73%     73%           
======================================
  Files         540     540           
  Lines       33465   33468    +3     
======================================
+ Hits        24436   24444    +8     
+ Misses       9029    9024    -5     
Impacted Files Coverage Δ
common/flights/src/store_client.rs 85% <66%> (-2%) ⬇️
common/functions/src/scalars/arithmetics/mod.rs 0% <0%> (ø)
store/src/meta_service/raftmeta.rs 93% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47510e7...bbf0e41. Read the comment docs.

@databend-bot
Copy link
Member

CI Passed
Reviewer Approved
Let's Merge

@databend-bot databend-bot merged commit 0aacd73 into datafuselabs:master Aug 27, 2021
@PsiACE PsiACE deleted the bump-tonic branch August 27, 2021 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-build this PR changes build/testing/ci steps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RUSTSEC-2021-0073: Conversion from prost_types::Timestamp to SystemTime can cause an overflow and panic
5 participants