Skip to content

Aggregated parameter implementation#130

Closed
todvora wants to merge 4 commits intomainfrom
feature/aggregated_param
Closed

Aggregated parameter implementation#130
todvora wants to merge 4 commits intomainfrom
feature/aggregated_param

Conversation

@todvora
Copy link
Contributor

@todvora todvora commented Feb 5, 2025

⚠️ Keeping for reference now, but I would not recommend merging it and rather use a different approach. This is too fragile, unpredictable and too much relying on conventions.

This PR implements following syntax for aggregated configuration properties:

@AggregatedParameter(prefix = "opensearch.", stripPrefix = true)
private Map<String, String> opensearchProperties;

Given following config properties file:

opensearch.node.roles=search,cluster_manager
opensearch.node.search.cache.size=10gb
opensearch.path.repo=/tmp

The map will be contain all three properties, with the prefix removed.

This is needed to support pass-through properties in the data node. All opensearch. prefixed properties will be forwarded to the opensearch process, without the need to explicitly support them in the data node.

Notes for Reviewers

  • The commit history must be preserved - please use the rebase-merge or standard merge option instead of squash-merge
  • Sync up with the author before merging

@todvora todvora force-pushed the feature/aggregated_param branch from 7c25ce5 to 6c983ea Compare February 6, 2025 10:21
@todvora todvora marked this pull request as ready for review February 6, 2025 10:52
Copy link

@moesterheld moesterheld left a comment

Choose a reason for hiding this comment

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

Thanks, this is what we need for easier pass-through of OpenSearch parameters to data node.
However, I think I found an error. Please also add readNames tests to the repository tests.

@todvora todvora requested a review from moesterheld February 7, 2025 14:21
@todvora todvora marked this pull request as draft February 21, 2025 11:05
@todvora todvora closed this Feb 28, 2025
@todvora
Copy link
Contributor Author

todvora commented Feb 28, 2025

We decided for a different, simpler and more reliable implementation directly in the data node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants