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-7730] HoodieStorage.openSeekable should not have wrapStream param #12774

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ktblsva
Copy link
Contributor

@ktblsva ktblsva commented Feb 5, 2025

Change Logs

Remove wrapStream param

Impact

Remove param which is not relevant to non-hadoop fs

Risk level (write none, low medium or high below)

NA

Documentation Update

None

Contributor's checklist

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

@ktblsva ktblsva marked this pull request as draft February 5, 2025 10:04
@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Feb 5, 2025
@hudi-bot
Copy link

hudi-bot commented Feb 5, 2025

CI report:

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

@ktblsva ktblsva marked this pull request as ready for review February 5, 2025 14:42
@wombatu-kun wombatu-kun requested a review from jonvex February 6, 2025 01:56
@danny0405
Copy link
Contributor

Remove param which is not relevant to non-hadoop fs

Are you saying the API for non-hadoop fs is not simple enough?

@ktblsva
Copy link
Contributor Author

ktblsva commented Feb 7, 2025

Remove param which is not relevant to non-hadoop fs

Are you saying the API for non-hadoop fs is not simple enough?

this is what was written in the task:

HoodieStorage.openSeekable is needed to wrap the input stream when reading log files in Hadoop. We had to specify this boolean value in the API, but should not do so as it will not be relevant to other file systems.

so yes, looks like wrapStream param needs only for read log files in hadoop and nowhere else

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S PR with lines of changes in (10, 100]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants