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.

Sign up for GitHub

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

Jump to bottom

Rqd config file from env var and copy rqd host env var #1270

Merged
merged 14 commits into from
May 3, 2024
Merged

Rqd config file from env var and copy rqd host env var #1270

merged 14 commits into from
May 3, 2024

Conversation

KernAttila
Copy link
Contributor

@KernAttila KernAttila commented Mar 14, 2023 •
edited
Loading

Link the Issue(s) this Pull Request is related to.

#574

Summarize your change.

RQD process being isolated from the system running it, we should be able to declare environment variables to be used in the frame command.

  • Added option to load rqd config file from an env variable RQD_CONFIG_FILE
  • Copy specific environment variable from the RQD host to the frame env when running the frame.
  • New section in config file : UseHostEnvVar
  • Added nimby variables CHECK_INTERVAL_LOCKED and MINIMUM_IDLE as configurable
  • OVERRIDE_IS_DESKTOP readable from the config file (desktop mode not detected on Window yet)
  • Added an example config file

Questions

  • I would appreciate some help to create proper tests for this feature.
  • Is there any hole in this logic of pulling env variable ?

@KernAttila
feat: load rqd config file from env var RQD_CONFIG_FILE
e7e0f88
@KernAttila
feat: new variable RQD_HOST_ENV_VARS used to copy env variables fro…
8a31671 
…m the submitter to the RQD worker
@KernAttila
feat: renamed variable __section to __override_section to accomod…
f448374 
…ate multiple sections

feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file
@KernAttila
feat: renamed variable __section to __override_section to accomod…
7978493 
…ate multiple sections

feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file
@KernAttila
feat: use the list of env var from RQD_HOST_ENV_VARS to copy them i…
d1ba72c 
…n the frame env.
@KernAttila
docs: fix misleading comment
0e51a0a
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 14, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@KernAttila KernAttila marked this pull request as ready for review March 15, 2023 18:12
@KernAttila KernAttila changed the title Rqd config file from env var Rqd config file from env var and copy rqd host env var Mar 16, 2023
DiegoTavares
DiegoTavares approved these changes Apr 24, 2023
View reviewed changes
Copy link
Collaborator

@DiegoTavares DiegoTavares left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

I think @bcipriano had mentioned something similar was in his Radar, but I couldn't find a PR for it, so IMO it's okay to merge as soon as the single comment I had gets addressed and some unit tests get added.

As for testing, something similar to this would be enough:
https://github.com/AcademySoftwareFoundation/OpenCue/blob/master/rqd/tests/rqconstants_tests.py

rqd/rqd.example.conf Show resolved Hide resolved
@KernAttila KernAttila mentioned this pull request Nov 9, 2023
Open
@angelali-ms angelali-ms mentioned this pull request Dec 8, 2023
Closed
@DiegoTavares DiegoTavares merged commit 8fd3d28 into AcademySoftwareFoundation:master May 3, 2024
12 checks passed
carlosfelgarcia pushed a commit to carlosfelgarcia/OpenCue that referenced this pull request May 22, 2024
@KernAttila @carlosfelgarcia
Rqd config file from env var and copy rqd host env var (AcademySoftwa…
87ee8e6 
…reFoundation#1270)

* feat: load rqd config file from env var `RQD_CONFIG_FILE`

* feat: new variable `RQD_HOST_ENV_VARS` used to copy env variables from the submitter to the RQD worker

* feat: renamed variable `__section` to `__override_section` to accomodate multiple sections
feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file

* feat: renamed variable `__section` to `__override_section` to accomodate multiple sections
feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file

* feat: use the list of env var from `RQD_HOST_ENV_VARS` to copy them in the frame env.

* docs: fix misleading comment

* doc: add example rqd.conf file

* doc: header for example config file

* fix: add newline

Signed-off-by: Kern Attila GERMAIN <[email protected]>

* doc: added a description for the new UseHostEnvVar section in the example file.

* doc: Added some notes about usage and rez alternative

* chores: pylint line too long

---------

Signed-off-by: Kern Attila GERMAIN <[email protected]>
carlosfelgarcia pushed a commit to carlosfelgarcia/OpenCue that referenced this pull request May 22, 2024
@KernAttila @carlosfelgarcia
Rqd config file from env var and copy rqd host env var (AcademySoftwa…
1c42c8f 
…reFoundation#1270)

* feat: load rqd config file from env var `RQD_CONFIG_FILE`

* feat: new variable `RQD_HOST_ENV_VARS` used to copy env variables from the submitter to the RQD worker

* feat: renamed variable `__section` to `__override_section` to accomodate multiple sections
feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file

* feat: renamed variable `__section` to `__override_section` to accomodate multiple sections
feat: added `__host_env_var_section`
feat: Allow config file to contain keys without value
feat: Respect case from the config file keys
fix: Added optionnal `OVERRIDE_IS_DESKTOP` from the config file (detection does not work on Windows so we have to force it somehow)
feat: Added `CHECK_INTERVAL_LOCKED` and `MINIMUM_IDLE` for nimby configuration
feat: added `RQD_HOST_ENV_VARS` for config file

* feat: use the list of env var from `RQD_HOST_ENV_VARS` to copy them in the frame env.

* docs: fix misleading comment

* doc: add example rqd.conf file

* doc: header for example config file

* fix: add newline

Signed-off-by: Kern Attila GERMAIN <[email protected]>

* doc: added a description for the new UseHostEnvVar section in the example file.

* doc: Added some notes about usage and rez alternative

* chores: pylint line too long

---------

Signed-off-by: Kern Attila GERMAIN <[email protected]>
@KernAttila KernAttila deleted the rqd-config-file-from-env-var branch August 21, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DiegoTavares DiegoTavares DiegoTavares approved these changes

@bcipriano bcipriano Awaiting requested review from bcipriano bcipriano is a code owner

@gregdenton gregdenton Awaiting requested review from gregdenton

@jrray jrray Awaiting requested review from jrray

@smith1511 smith1511 Awaiting requested review from smith1511

@larsbijl larsbijl Awaiting requested review from larsbijl

@IdrisMiles IdrisMiles Awaiting requested review from IdrisMiles

@splhack splhack Awaiting requested review from splhack

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KernAttila @DiegoTavares