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

Fix rpm build by adding config to env variable #172

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

sruti1312
Copy link
Contributor

@sruti1312 sruti1312 commented Apr 27, 2022

Signed-off-by: sruti1312 srutiparthiban@gmail.com

Is your feature request related to a problem? Please provide an existing Issue # , or describe.
opensearch-project/performance-analyzer#197

Describe the solution you are proposing
Checks for opensearch.path.conf before setting the default config value.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>
@sruti1312 sruti1312 requested review from a team, dzane17 and sgup432 April 27, 2022 21:45
@sruti1312 sruti1312 merged commit 00eac3f into main Apr 27, 2022
@sruti1312 sruti1312 deleted the feature/update-configPath branch April 27, 2022 22:23
Comment on lines +26 to +34
public static final String DEFAULT_CONFIG_FOLDER_PATH =
OPENSEARCH_HOME
+ File.separator
+ "config"
+ File.separator
+ "opensearch-performance-analyzer"
+ File.separator
+ "performance-analyzer.properties";
+ File.separator;
private static final String DEFAULT_CONFIG_FILE_PATH =
DEFAULT_CONFIG_FOLDER_PATH + "performance-analyzer.properties";
Copy link
Member

Choose a reason for hiding this comment

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

Are we still using OPENSEARCH_HOME here for the default config folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using OPENSEARCH_HOME here for the default config folder. Removing the default maybe a better idea. Any thoughts?

performance-analyzer-agent which starts the PA process will never succeed if the setting is not present, which means this DEFAULT value will never be used.

Copy link
Member

Choose a reason for hiding this comment

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

I am ok to remove the var if never used.

sruti1312 added a commit that referenced this pull request Apr 28, 2022
* Update configFilePath location (#172)

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>

* Disable plugin if configFilePath is not present

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>
sruti1312 added a commit that referenced this pull request May 11, 2022
Signed-off-by: sruti1312 <srutiparthiban@gmail.com>
sruti1312 added a commit that referenced this pull request May 17, 2022
* Update folder names (#166)

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>

* Update PA directories from plugins to root (#168)

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>

* Update configFilePath location (#172)

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>

* Disable plugin if configFilePath is not present (#173)

Signed-off-by: sruti1312 <srutiparthiban@gmail.com>
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.

4 participants