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

[HUDI-7367] Add makeQualified APIs #10607

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

yihua
Copy link
Contributor

@yihua yihua commented Feb 2, 2024

Change Logs

This PR makes the following changes to properly support qualified location:

  • Adds getUri to HoodieStorage.
  • Adds makeQualified to HoodieLocation.
  • Adds makeQualified to FSUtils.
  • New tests for the functionality.

Impact

Properly support qualified location with HoodieStorage and HoodieLocation. This fixes the original test failures in #10591.

Risk level

none

Documentation Update

N/A

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed

@yihua
Copy link
Contributor Author

yihua commented Feb 3, 2024

@linliu-code to review

@hudi-bot
Copy link

hudi-bot commented Feb 3, 2024

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@vinothchandar vinothchandar self-assigned this Feb 6, 2024
Copy link
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

Minor comments. Willing to concede for now, given its matching the hadoop APIs closely

if (!isAbsolute()) {
throw new IllegalStateException("Only an absolute path can be made qualified");
}
HoodieLocation location = this;
Copy link
Member

Choose a reason for hiding this comment

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

found this a bit weird. the assignment from this. is that needed?

* a new path that includes a path and authority and is fully qualified.
*/
@PublicAPIMethod(maturity = ApiMaturityLevel.EVOLVING)
public HoodieLocation makeQualified(URI defaultUri) {
Copy link
Member

Choose a reason for hiding this comment

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

this ought to be a static method? I think we should instead make this a new constructor in HoodieLocation?

@vinothchandar vinothchandar merged commit 395986e into apache:master Feb 7, 2024
29 of 31 checks passed
yihua added a commit that referenced this pull request Feb 27, 2024
* [HUDI-7367] Add makeQualified APIs

* Fix checkstyle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants