Skip to content

Conversation

@coryan
Copy link
Contributor

@coryan coryan commented Nov 29, 2021

No description provided.

@github-actions
Copy link

@coryan coryan marked this pull request as ready for review November 29, 2021 16:28
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM, just a suggestion

result = fs->OpenInputStream(NotFoundObjectPath());
EXPECT_EQ(result.status().code(), StatusCode::IOError);
}

Copy link
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?

Copy link
Contributor Author

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)

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@pitrou pitrou closed this in bd4d1e6 Nov 30, 2021
@ursabot
Copy link

ursabot commented Nov 30, 2021

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants