Skip to content

[HUDI-7871] remove tableconfig from filegroup reader params#11449

Merged
jonvex merged 1 commit intoapache:masterfrom
jonvex:remove_tableconfig_from_fg_reader_params
Jun 13, 2024
Merged

[HUDI-7871] remove tableconfig from filegroup reader params#11449
jonvex merged 1 commit intoapache:masterfrom
jonvex:remove_tableconfig_from_fg_reader_params

Conversation

@jonvex
Copy link
Contributor

@jonvex jonvex commented Jun 13, 2024

Change Logs

HoodieFileGroupReader has too many params.
We can get the tableconfig from the metaclient which is also a param.

Impact

Less params for fg reader

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

low

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

@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Jun 13, 2024
Option<InternalSchema> internalSchemaOpt,
HoodieTableMetaClient hoodieTableMetaClient,
TypedProperties props,
HoodieTableConfig tableConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you think about simplifying all the parameters? The goal should be that given the file group ID and the query type with the reader context, storage instance, and the minimal set of configs (maybe with meta client), the file group reader should be able to figure out all necessary configs to fill in for reading the records out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can keep getting rid of more. Just want to keep the prs small

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@apache apache deleted a comment from hudi-bot Jun 13, 2024
@hudi-bot
Copy link
Collaborator

CI report:

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

@jonvex jonvex merged commit 8f0467b into apache:master Jun 13, 2024
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.

5 participants