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.

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

Move path resolution logic and fix a bug #18146

Merged
merged 4 commits into from
Sep 14, 2023
Merged

Move path resolution logic and fix a bug #18146

merged 4 commits into from
Sep 14, 2023

Conversation

jiacheliu3
Copy link
Contributor

@jiacheliu3 jiacheliu3 commented Sep 13, 2023 •
edited
Loading

What changes are proposed in this pull request?

  1. Move the path resolution logic from DoraCacheFileSystem to PathUtils where it makes more sense
  2. Fix the alluxioPathToUfsPath resolution by handling the case where the ufs path may have no matching alluxio path, making the util method more generic

@alluxio-bot
Copy link
Contributor

Automated checks report:

  • PR title follows the conventions: FAIL
    • The title of the PR does not pass all the checks. Please fix the following issues:
      • First word must be capitalized
  • Commits associated with Github account: PASS

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

@jiacheliu3 jiacheliu3 changed the title move util to a util class Move path resolution logic and fix a bug Sep 13, 2023
@alluxio-bot
Copy link
Contributor

Automated checks report:

  • PR title follows the conventions: PASS
  • Commits associated with Github account: PASS

All checks passed!

@jiacheliu3 jiacheliu3 requested a review from dbw9580 September 13, 2023 15:36
@jiacheliu3 jiacheliu3 added the type-code-quality code quality improvement label Sep 13, 2023
dbw9580
dbw9580 approved these changes Sep 14, 2023
View reviewed changes
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
Copy link
Contributor Author

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 876d57f into main Sep 14, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbw9580 dbw9580 dbw9580 approved these changes

Assignees
No one assigned
Labels
type-code-quality code quality improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jiacheliu3 @alluxio-bot @dbw9580