Skip to content

ARROW-14912: [C++] implement GcsFileSystem::CopyFile #11797

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.

Sign up for GitHub

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

Jump to bottom
Closed
coryan wants to merge 2 commits into from
Closed

ARROW-14912: [C++] implement GcsFileSystem::CopyFile #11797

coryan wants to merge 2 commits into from

Conversation

@coryan
Copy link
Contributor

@coryan coryan commented Nov 30, 2021

No description provided.

@github-actions
Copy link

github-actions bot commented Nov 30, 2021

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

@coryan
Copy link
Contributor Author

coryan commented Nov 30, 2021

The Integration / AMD64 Conda Integration Test failure seems unrelated and may be a transient failure:

Error:  Failed to execute goal on project arrow-java-root: Could not resolve dependencies for project org.apache.arrow:arrow-java-root:pom:7.0.0-SNAPSHOT: The following artifacts could not be resolved: org.slf4j:log4j-over-slf4j:jar:1.7.25, org.junit.platform:junit-platform-runner:jar:1.4.0: Could not transfer artifact org.slf4j:log4j-over-slf4j:jar:1.7.25 from/to central (https://repo.maven.apache.org/maven2): Connection reset -> [Help 1]

@coryan coryan marked this pull request as ready for review November 30, 2021 02:10
pitrou
pitrou reviewed Nov 30, 2021
View reviewed changes
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.

Thank you for doing this. LGTM, just a suggestion.

cpp/src/arrow/filesystem/gcsfs_test.cc Outdated
auto fs = internal::MakeGcsFileSystemForTest(TestGcsOptions());
const auto destination_path = kPreexistingBucket + std::string("/copy-destination");
const auto status = fs->CopyFile(NotFoundObjectPath(), destination_path);
EXPECT_EQ(status.code(), StatusCode::IOError);
Copy link
Member

@pitrou pitrou Nov 30, 2021

Choose a reason for hiding this comment

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

Nit: this can be shortened to:

ASSERT_RAISES(IOError, fs->CopyFile(NotFoundObjectPath(), destination_path));

If the error message is predictable enough, you could also test for it using e.g.:

EXPECT_RAISES_WITH_MESSAGE_THAT(IOError, ::testing::HasSubstr("xxx"),
    fs->CopyFile(NotFoundObjectPath(), destination_path));

Copy link
Contributor Author

@coryan coryan Nov 30, 2021

Choose a reason for hiding this comment

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

Done. PTAL.

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

ursabot commented Nov 30, 2021 •
edited
Loading

Benchmark runs are scheduled for baseline = 8daa7a4 and contender = 996a08b. 996a08b 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 ⬇️0.45% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.18% ⬆️0.04%] 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-14912-implement-gcs-file-system-copy-file branch November 30, 2021 14:31
@asfimport asfimport mentioned this pull request Dec 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou approved these changes

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coryan @ursabot @pitrou