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

[OSCI][DEV]ingest feature flags from .env file #10544

Closed

Conversation

kohinoor98
Copy link

@kohinoor98 kohinoor98 commented Oct 10, 2023

Description

Reads the .env file and then applies the necessary flags to the system instead of passing in flags in the commandline.

Related Issues

Resolves #594

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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: kohinoor98 <kohinoorchatterjee1998@gmail.com>
Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

…into 594_modify_config

Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock
Copy link
Member

dblock commented Oct 23, 2023

Ideally, I'd like to be able to do both OPENSEARCH_XYZ=123 or dotenv gradle run .... This change does allow me to do (2) - is it good enough? Maybe others have some thoughts? @peternied WDYT?

@kohinoor98

  1. You should iterate to green either way.
  2. Is (1) somehow possible?

@kohinoor98
Copy link
Author

kohinoor98 commented Oct 23, 2023

Hi @dblock

  1. Please could you help me understand how I could achieve:
    You should iterate to green either way

  2. For the command:

dotenv ./gradlew run

dotenv-cli would need to be installed on the users system.

This tool helps manage the environment variables by loading them from a .env file into the ENV of your script's process. However, in the context of the changes, whether or not dotenv is used explicitly to override flags doesn't make a difference. This is because, during the startup script execution, environment variables will be picked up directly, making the commands dotenv ./gradlew run and ./gradlew run effectively identical in behavior. The system inherently reads env variables, ensuring no disparity in the execution process resulting from how they were initiated.

Therefore, while dotenv-cli aids in the initial loading of your custom environment variables, the actual running script initiated by gradlew remains unaffected by this intermediary.

  1. (1) should be possible(could impact multiple points throughout; would need to experiment to know more) but I believe it is already in use as snakeyaml is being used and the flags from yaml are being ingested throughout the project like in this PR. Please advise on next steps on this.

Thanks,
KC

@dblock
Copy link
Member

dblock commented Oct 25, 2023

dotenv-cli would need to be installed on the users system.

This is totally normal, typical use of dotenv.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

I am not sure what the value of having a .env file is vs opensearch.yml, but maybe I am not the developer targeted by this effort 🤷

I develop on my less-than-beefy windows laptop by using VSCode's remote plugin to an EC2 instance does the heavy lifting, maybe I am part of the audience but have been worked around these issues addressed with .env?

PS: I rarely use ./gradlew run opting to use test cases to validate what I'm build most of the time.

Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
@github-actions github-actions bot added Build Libraries & Interfaces enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source. Priority-Low labels Nov 2, 2023
Copy link
Contributor

github-actions bot commented Nov 2, 2023

Gradle Check (Jenkins) Run Completed with:

@kohinoor98
Copy link
Author

Hi @dblock

Gradle Check is failing due to a known flaky-test addressed here #7396.

I think rest is good to go. Please let me know if anything else has to be done for this PR.

Thanks a lot,
KC

@dblock
Copy link
Member

dblock commented Nov 16, 2023

@kohinoor98 Please do rebase/force push an update to re-trigger tests until they pass. Appreciate any time you can spend fixing flaky tests while you wait on retries, too.

Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
Copy link
Contributor

❌ Gradle check result for 79150da: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: kohinoor98 <kohinoorchatterjee1998@gmail.com>
Copy link
Contributor

❌ Gradle check result for 3fea373: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Dec 20, 2023
@stephen-crawford
Copy link
Contributor

I suggest we close this. @kohinoor98 you are more than welcome to open a new PR for this change in the future.

@kohinoor98
Copy link
Author

I suggest we close this. @kohinoor98 you are more than welcome to open a new PR for this change in the future.

I am facing some issues with hitting all green due to some configurations. I will create a new PR for the same.

@kohinoor98 kohinoor98 closed this Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Libraries & Interfaces enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source. Priority-Low stalled Issues that have stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DEV] Allow developers to have a local/modify opensearch.yml or ENV
4 participants