Skip to content

Fix rpm build by adding config to env variable#172

Merged
sruti1312 merged 1 commit intomainfrom
feature/update-configPath
Apr 27, 2022
Merged

Fix rpm build by adding config to env variable#172
sruti1312 merged 1 commit intomainfrom
feature/update-configPath

Conversation

@sruti1312
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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