-
Notifications
You must be signed in to change notification settings - Fork 2.5k
refactor: Add Lombok annotations to hudi-spark,hudi-spark-common #17718
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
Conversation
e60079a to
33ce24e
Compare
33ce24e to
d1d8220
Compare
33b949e to
34a619b
Compare
CTTY
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! There are some places where we can apply parameterized logging, would be good if we can address them
| * Note: Explicit setters are used instead of Lombok's @Setter annotation because this class is accessed from Scala code | ||
| * (RunBootstrapProcedure.scala). Since Scala compilation happens before Java compilation in the Maven build lifecycle, | ||
| * Lombok-generated methods would not be visible to the Scala compiler, causing compilation errors. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
| this.props = this.propsFilePath == null || this.propsFilePath.isEmpty() ? buildProperties(this.configs) | ||
| : readConfig(fs.getConf(), new StoragePath(this.propsFilePath), this.configs).getProps(true); | ||
| LOG.info("Starting data import with configs : " + props.toString()); | ||
| log.info("Starting data import with configs : " + props.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can apply parameterized logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| } | ||
| this.bootstrapConfig = builder.build(); | ||
| LOG.info("Created bootstrap executor with configs : " + bootstrapConfig.getProps()); | ||
| log.info("Created bootstrap executor with configs : " + bootstrapConfig.getProps()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can apply parameterized logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| writer.save(tablePath); // ultimately where the dataset will be placed | ||
| String commitInstantTime1 = HoodieDataSourceHelpers.latestCommit(fs, tablePath); | ||
| LOG.info("First commit at instant time :" + commitInstantTime1); | ||
| log.info("First commit at instant time :" + commitInstantTime1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can apply parameterized logging to some places in this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| private DataFrameWriter<Row> updateHiveSyncConfig(DataFrameWriter<Row> writer) { | ||
| if (enableHiveSync) { | ||
| LOG.info("Enabling Hive sync to " + hiveJdbcUrl); | ||
| log.info("Enabling Hive sync to " + hiveJdbcUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| FileSystem fs = FileSystem.get(jssc.hadoopConfiguration()); | ||
| String commitInstantTime1 = HoodieDataSourceHelpers.latestCommit(fs, tablePath); | ||
| LOG.info("Commit at instant time :" + commitInstantTime1); | ||
| log.info("Commit at instant time :" + commitInstantTime1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| waitTillNCommits(fs, numExpCommits, 180, 3); | ||
| commitInstantTime2 = HoodieDataSourceHelpers.listCommitsSince(fs, tablePath, commitInstantTime1).stream().sorted().findFirst().get(); | ||
| LOG.info("Second commit at instant time :" + commitInstantTime2); | ||
| log.info("Second commit at instant time :" + commitInstantTime2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can apply parameterized logging to some places in this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
| // Ensure all commits were synced to the Metadata Table | ||
| HoodieTableMetaClient metadataMetaClient = createMetaClient(metadataTableBasePath); | ||
| LOG.warn("total commits in metadata table " + metadataMetaClient.getActiveTimeline().getCommitsTimeline().countInstants()); | ||
| log.warn("total commits in metadata table " + metadataMetaClient.getActiveTimeline().getCommitsTimeline().countInstants()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can apply parameterized logging to some places in this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Describe the issue this Pull Request addresses
This PR refactors the
hudi-spark-datasource/hudi-sparkandhudi-spark-datasource/hudi-spark-commonmodules to reduce boilerplate code by leveraging Project Lombok annotations. Specifically, it replaces explicitLoggerinstantiation, manual getter/setter methods, and empty constructors with their equivalent Lombok annotations (@Slf4j,@Getter,@Setter,@NoArgsConstructor,@AllArgsConstructor,@Data,@Value,@ToString).This improves code readability and maintainability without altering the runtime logic.
Summary and Changelog
This change introduces the Lombok dependency to the
hudi-spark-datasource/hudi-sparkandhudi-spark-datasource/hudi-spark-commonmodules and refactors several classes to utilize Lombok annotations.hudi-spark-datasource/hudi-sparkandhudi-spark-datasource/hudi-spark-commonmodules.Impact
Risk Level
none
(This is a pure refactoring change involving standard library annotations; no business logic was modified.)
Documentation Update
none
Contributor's checklist