Skip to content

HDDS-11992. Replace GenericCli#createOzoneConfiguration calls with getOzoneConf.#7623

Merged
adoroszlai merged 2 commits intoapache:masterfrom
nandakumar131:HDDS-11992
Dec 29, 2024
Merged

HDDS-11992. Replace GenericCli#createOzoneConfiguration calls with getOzoneConf.#7623
adoroszlai merged 2 commits intoapache:masterfrom
nandakumar131:HDDS-11992

Conversation

@nandakumar131
Copy link
Contributor

What changes were proposed in this pull request?

Replace GenericCli#createOzoneConfiguration calls with GenericCli#getOzoneConf.

GenericCli#getOzoneConf is getting called before picocli injects the command-line Options configurationOverrides and configurationPath, so the OzoneConfigutation object is created before picocli updates these Options.

To update the OzoneConfigutation object with configurationOverrides and configurationPath, this PR changes the configurationOverrides and configurationPath instance variables to methods.

What is the link to the Apache JIRA

HDDS-11992

How was this patch tested?

CI Run: 12523963928

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

GenericCli#getOzoneConf is getting called before picocli injects the command-line Options configurationOverrides and configurationPath, so the OzoneConfigutation object is created before picocli updates these Options.

Thanks for spotting it. For the record: this happens in execute(String[]) overridden to start tracing.

Comment on lines 57 to 53
private final Supplier<OzoneConfiguration> configSupplier =
MemoizedSupplier.valueOf(OzoneConfiguration::new);
private final CommandLine cmd;
private OzoneConfiguration conf;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since almost all commands use the config, I think we can create the config eagerly:

  private final OzoneConfiguration conf = new OzoneConfiguration();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Will update the PR.

@adoroszlai adoroszlai added the tools Tools that helps with debugging label Dec 28, 2024
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @nandakumar131 for updating the patch.

@adoroszlai adoroszlai merged commit c3003fd into apache:master Dec 29, 2024
42 checks passed
@nandakumar131 nandakumar131 deleted the HDDS-11992 branch December 30, 2024 05:03
@nandakumar131
Copy link
Contributor Author

Thanks for the review @adoroszlai!

@adoroszlai adoroszlai added command-line and removed tools Tools that helps with debugging labels Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments