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

Bump deps to upgrade tonic / prost / prost-derive #864

Merged
merged 8 commits into from
Nov 2, 2021
Merged

Bump deps to upgrade tonic / prost / prost-derive #864

merged 8 commits into from
Nov 2, 2021

Conversation

PsiACE
Copy link
Member

@PsiACE PsiACE commented Oct 26, 2021

Which issue does this PR close?

Closes #897

Rationale for this change

prost and tonic have recently had some updates

What changes are included in this PR?

  • bump deps, like prost 0.9.0, tonic 0.6.0
  • fix some error
  • change to lastest cargo-tarpaulin

Are there any user-facing changes?

I guess no changes

Signed-off-by: Chojan Shang <psiace@outlook.com>
Signed-off-by: Chojan Shang <psiace@outlook.com>
@github-actions github-actions bot added the arrow-flight Changes to the arrow-flight crate label Oct 26, 2021
@PsiACE PsiACE marked this pull request as draft October 26, 2021 03:39
@PsiACE
Copy link
Member Author

PsiACE commented Oct 26, 2021

Integration Test failed

archery docker run -e ARCHERY_INTEGRATION_WITH_RUST=1 conda-integration

tonic v0.6.0 need rust 1.56+ (edition2021), but it looks https://github.com/apache/arrow/blob/68a081acc4d0e2c85a6469ca165444df6d965d2b/ci/docker/conda-integration.dockerfile#L42 only get 1.55

cc @alamb @nevi-me , I am not familiar with conda-integration, maybe you guys can help me

@PsiACE
Copy link
Member Author

PsiACE commented Oct 26, 2021

also, we need to reset the cache of coverage (cargo and target).

Done.

Signed-off-by: Chojan Shang <psiace@outlook.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2021

Codecov Report

Merging #864 (aa6251a) into master (1f07735) will decrease coverage by 0.15%.
The diff coverage is 0.00%.

❗ Current head aa6251a differs from pull request most recent head 59dfd6d. Consider uploading reports for the commit 59dfd6d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
- Coverage   82.46%   82.30%   -0.16%     
==========================================
  Files         168      168              
  Lines       48175    48093      -82     
==========================================
- Hits        39726    39585     -141     
- Misses       8449     8508      +59     
Impacted Files Coverage Δ
arrow-flight/src/arrow.flight.protocol.rs 0.00% <ø> (ø)
...ng/src/flight_server_scenarios/auth_basic_proto.rs 0.00% <0.00%> (ø)
arrow/src/array/transform/fixed_binary.rs 75.00% <0.00%> (-3.95%) ⬇️
parquet/src/schema/visitor.rs 67.10% <0.00%> (-2.64%) ⬇️
arrow/src/compute/kernels/cast_utils.rs 87.50% <0.00%> (-2.50%) ⬇️
parquet/src/encodings/decoding.rs 90.45% <0.00%> (-2.02%) ⬇️
arrow/src/datatypes/ffi.rs 70.85% <0.00%> (-2.02%) ⬇️
parquet/src/arrow/arrow_reader.rs 89.32% <0.00%> (-1.95%) ⬇️
arrow/src/ffi.rs 84.69% <0.00%> (-1.89%) ⬇️
arrow/src/csv/reader.rs 90.00% <0.00%> (-1.78%) ⬇️
... and 71 more

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 1f07735...59dfd6d. Read the comment docs.

@PsiACE PsiACE marked this pull request as ready for review October 26, 2021 15:11
@PsiACE
Copy link
Member Author

PsiACE commented Oct 27, 2021

https://github.com/PsiACE/arrow-rs/runs/4022900917?check_suite_focus=true

re-run Integration Test , and pass all tests

Integration Test failed

archery docker run -e ARCHERY_INTEGRATION_WITH_RUST=1 conda-integration

tonic v0.6.0 need rust 1.56+ (edition2021), but it looks https://github.com/apache/arrow/blob/68a081acc4d0e2c85a6469ca165444df6d965d2b/ci/docker/conda-integration.dockerfile#L42 only get 1.55

https://issues.apache.org/jira/browse/ARROW-14489 , thanks @jimexist

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @PsiACE -- this looks great

@tustvold tustvold mentioned this pull request Nov 1, 2021
@alamb
Copy link
Contributor

alamb commented Nov 1, 2021

So since these are non minor version updates I think this is basically a non-backwards compatible set of changes -- so I can't backport them to the arrow 6.x line, but they'll be released in 7.0

I merged master to make sure that this is still good and will merge if it passes

@alamb alamb added the api-change Changes to the arrow API label Nov 1, 2021
@alamb
Copy link
Contributor

alamb commented Nov 1, 2021

MIRI failure tracked by #879

@alamb
Copy link
Contributor

alamb commented Nov 1, 2021

Merged again to see if we can get a clean passing run

@alamb alamb merged commit 1d3d5e3 into apache:master Nov 2, 2021
@alamb
Copy link
Contributor

alamb commented Nov 2, 2021

Thanks again @PsiACE

@alamb alamb changed the title Bump deps Bump deps to upgrade tonic / prost / prost-derive Nov 9, 2021
alamb added a commit to alamb/arrow-rs that referenced this pull request Nov 11, 2021
* Bump deps

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

* Setup lastest cargo-tarpaulin

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

* Try to use the lastest cargo

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

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
@PsiACE PsiACE deleted the bump-deps branch November 16, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change Changes to the arrow API arrow-flight Changes to the arrow-flight crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update prost and tonic
3 participants