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

Add Unit Test for OSS, OBS and GCS #17985

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Conversation

zhezhidashi
Copy link
Contributor

@zhezhidashi zhezhidashi commented Aug 14, 2023

What changes are proposed in this pull request?

Add Unit Test for OSS, OBS and GCS

Why are the changes needed?

Unit test is important for improving functions of Alluxio.

Does this PR introduce any user facing changes?

No

Copy link
Contributor

@JiamingMai JiamingMai left a comment

Choose a reason for hiding this comment

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

LGTM

@JiamingMai
Copy link
Contributor

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 71fd8c4 into Alluxio:main Sep 18, 2023
} catch (ServiceException e) {
LOG.error("Failed to create GCSUnderFileSystem.", e);
throw Throwables.propagate(e);
if (checkGCSCredentials(conf)) {
Copy link
Contributor

@jja725 jja725 Sep 20, 2023

Choose a reason for hiding this comment

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

this is not correct, we can't check whether this is set or not since only v1 would set access key and if we use v2 credential we would not be able to create ufs @JiamingMai @zhezhidashi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I will fix this later.

@zhezhidashi zhezhidashi deleted the UT-OSS-OBS-GCS branch September 28, 2023 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-code-quality code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants