Skip to content

ARROW-14899: [C++] implement GcsInputStream::GetMetadata - #11791

Closed
coryan wants to merge 2 commits into
apache:masterfrom
coryan:ARROW-14899-implement-gcs-read-metadata
Closed

ARROW-14899: [C++] implement GcsInputStream::GetMetadata#11791
coryan wants to merge 2 commits into
apache:masterfrom
coryan:ARROW-14899-implement-gcs-read-metadata

Conversation

@coryan

@coryancoryan commented Nov 29, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actionsBot commented Nov 29, 2021

Copy link
Copy Markdown

https://issues.apache.org/jira/browse/ARROW-14899

@coryan
coryan marked this pull request as ready for review November 29, 2021 16:28
pitrou
pitrou approved these changes Nov 30, 2021
View reviewed changes

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just a suggestion

Comment threadcpp/src/arrow/filesystem/gcsfs_test.cc
@@ -352,6 +360,68 @@ TEST_F(GcsIntegrationTest, ReadObjectInfoInvalid) {
EXPECT_EQ(result.status().code(), StatusCode::IOError);
}

@pitroupitrouNov 30, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it also possible to add a simple roundtrip test between ToObjectMetadata and FromObjectMetadata?

@coryancoryanNov 30, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done (with some limitations, not all fields have public modifiers in gcs::ObjectMetadata)

pitrou
pitrou approved these changes Nov 30, 2021
View reviewed changes

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the update!

@ursabot

ursabot commented Nov 30, 2021•
edited
Loading

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 996a08b and contender = bd4d1e6. bd4d1e6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️6.73% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.09% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

@coryan
coryan deleted the ARROW-14899-implement-gcs-read-metadata branch November 30, 2021 16:01
@asfimportasfimport mentioned this pull request Dec 1, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitroupitroupitrou approved these changes
+1 more reviewer
@scotthartscotthartscotthart approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@coryan@ursabot@pitrou@scotthart