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

Deprecate EXEC_ENV #1882

Closed
jondequinor opened this issue Aug 27, 2019 · 4 comments · Fixed by #9690
Closed

Deprecate EXEC_ENV #1882

jondequinor opened this issue Aug 27, 2019 · 4 comments · Fixed by #9690
Labels
release-notes:breaking-change Automatically categorise as breaking change in release notes

Comments

@jondequinor
Copy link
Contributor

Its only user, RMS, uses it to shadow PYTHONPATH for RMS jobs. RMS run method should instead get PYTHONPATH as the eight argument.

@markusdregi
Copy link
Contributor

I think we should at least consider executing this issue.

@hnformentin hnformentin self-assigned this Jun 25, 2020
@jondequinor
Copy link
Contributor Author

jondequinor commented Jul 15, 2020

EXEC_ENV. Sounds generic. Sounds potent! But, alas, it is neither.

This is probably how EXEC_ENV is used today:

EXEC_ENV {"foo": null, "bar": "qux"}

is entered by the user into an ERT configuration. This is then, by job_dispatch.py—on the cluster or locally, depending on queue configuration—dumped into the runpath as {executable}_exec_env.json.

rms_run.py then reads this file and applies it for some RMS executable. The environment of that executable will be manipulated as follows:

  • if foo exists in the environment, it is removed
  • bar is set to qux

So, is it used for any other "environments" or "executables" like ert itself, workflows or other official forward models? No, not as far as I can see.

Can it do special thing to the "execution" "environment"? No, it sets or unsets variables. It's run-of-the-mill os.environ jiggling.

Do users write their own forward models that picks up {executable}_exec_env.json? If you write your own forward model, why wouldn't you just manipulate the environment directly?

It seems like a really bad name for a very specific RMS feature.

For developers, it is odd that, if the above is true, that it's so prominent in the job_dispatch.py and related code. The developer would think the job_runner used by job_dispatch.py system should strive to be generic, but currently it has a bunch of code dealing specifically with EXEC_ENV. So, from a ERT developer point of view, EXEC_ENV is carried up until execution of the forward model, but the buck kind of stops there. Nothing except the RMS FM will care about this json file.

If the code is not generic, it should be removed or replaced in favor of something generic. Can we make EXEC_ENV generic? Should we, in all our forward models, read and manipulate the environment as per EXEC_ENV? Probably not.

We could deprecate EXEC_ENV, suggest that the user copies a pre-populated {executable}_exec_env.json with COPY_FILE whenever it's required by an RMS job.

If this is tricky, we could provide the user with a new configuration option called UPDATE_FORWARD_MODEL_ENVIRONMENT which takes a forward model and some chunk of data to capture the intent.

UPDATE_FORWARD_MODEL_ENVIRONMENT rms_job_1 {"add_keys": [{"bar": "qux"}, …], "remove_keys": ["foo"]}

This would not surprise anyone.

@oyvindeide
Copy link
Collaborator

@sondreso sondreso added this to SCOUT Jan 3, 2025
@sondreso sondreso moved this to Todo in SCOUT Jan 3, 2025
@berland
Copy link
Contributor

berland commented Jan 9, 2025

Blocked by: equinor/rms-sys#208

Unblocked now.

@berland berland mentioned this issue Jan 9, 2025
9 tasks
@berland berland moved this from Todo to Ready for Review in SCOUT Jan 9, 2025
@berland berland added the release-notes:breaking-change Automatically categorise as breaking change in release notes label Jan 9, 2025
@andreas-el andreas-el moved this from Ready for Review to Reviewed in SCOUT Jan 9, 2025
@github-project-automation github-project-automation bot moved this from Reviewed to Done in SCOUT Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:breaking-change Automatically categorise as breaking change in release notes
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants