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

Extract path resolution in DoraCacheFS into static utility #18140

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

jiacheliu3
Copy link
Contributor

@jiacheliu3 jiacheliu3 commented Sep 13, 2023

What changes are proposed in this pull request?

In this change the path conversion logic is extracted to static utility methods for code reuse (because other classes may use the same path resolution logic).

The method names are slightly improved, to distinguish the member methods in DoraCacheFileSystem (may be inherited) from the static utility methods.

@jiacheliu3 jiacheliu3 requested a review from dbw9580 September 13, 2023 08:29
@jiacheliu3 jiacheliu3 changed the title Extract utility Extract path resolution in DoraCacheFS into static utility Sep 13, 2023
}

// Treat this path as Alluxio relative, and add the UFS root before it.
String ufsFullPath = PathUtils.concatPath(ufsRootPath, alluxioPath.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
String ufsFullPath = PathUtils.concatPath(ufsRootPath, alluxioPath.toString());
String ufsFullPath = PathUtils.concatPath(ufsRootPath, alluxioPath.getPath());

if (alluxioPath.isRoot()) {
ufsFullPath = ufsFullPath + AlluxioURI.SEPARATOR;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this supposed to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't know, this is a refactor

@jiacheliu3
Copy link
Contributor Author

The only errors are

2023-09-13T09:50:53.9653777Z 09:50:53.964 [ERROR] Errors: 
2023-09-13T09:50:53.9654372Z 09:50:53.964 [ERROR] alluxio.underfs.oss.StsOssClientProviderTest.testInitAndRefresh
2023-09-13T09:50:53.9655829Z 09:50:53.964 [ERROR]   Run 1: StsOssClientProviderTest.testInitAndRefresh:77 » Runtime No value set for configuration key alluxio.underfs.oss.proxy.host
2023-09-13T09:50:53.9656927Z 09:50:53.964 [ERROR]   Run 2: StsOssClientProviderTest.testInitAndRefresh:77 » Runtime No value set for configuration key alluxio.underfs.oss.proxy.host

Copy link
Contributor

@dbw9580 dbw9580 left a comment

Choose a reason for hiding this comment

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

LGTM

@jiacheliu3 jiacheliu3 added the type-code-quality code quality improvement label Sep 13, 2023
@jiacheliu3
Copy link
Contributor Author

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 7328331 into main Sep 13, 2023
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.

3 participants