Prevent setting LSF_SERVER to an undefined environment variable #6051
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
larsevj
force-pushed
the
prevent_empty_LSF_SERVER_env
branch
from
0bd793b
to
c13f899
Compare
Codecov Report
@@ Coverage Diff @@
## main #6051 +/- ##
==========================================
+ Coverage 82.44% 82.47% +0.03%
==========================================
Files 350 350
Lines 21431 21433 +2
Branches 839 839
==========================================
+ Hits 17668 17677 +9
+ Misses 3465 3458 -7
Partials 298 298
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
larsevj
force-pushed
the
prevent_empty_LSF_SERVER_env
branch
from
01baf38
to
5e4c73c
Compare
oysteoh
reviewed
Sep 19, 2023
src/ert/config/queue_config.py
Outdated
| if option_name == "LSF_SERVER" and values[0].startswith("$"): | ||
| raise ConfigValidationError( | ||
| f"Invalid server name for QUEUE_OPTION LSF LSF_SERVER: " | ||
| f"{values[0]}. Check for empty enviroment variable. " |
There was a problem hiding this comment.
enviroment -> environment
"The LSF_SERVER is usually specified in the site-config file. " - maybe more like something like this?
"The LSF_SERVER value is usually provided by default in a site-configuration file. If you have no special needs you can safely remove it from your configuration"
larsevj
force-pushed
the
prevent_empty_LSF_SERVER_env
branch
2 times, most recently
from
d7d37e4
to
a75c648
Compare
larsevj
force-pushed
the
prevent_empty_LSF_SERVER_env
branch
from
a75c648
to
b9dcb76
Compare
larsevj
added
the
release-notes:maintenance
larsevj
changed the title
andreas-el
reviewed
Sep 25, 2023
src/ert/config/queue_config.py
Outdated
| if option_name == "LSF_SERVER" and values[0].startswith("$"): | ||
| raise ConfigValidationError( | ||
| "Invalid server name for QUEUE_OPTION LSF LSF_SERVER:" | ||
| f" {values[0]}. Check for empty environment variable. The" |
There was a problem hiding this comment.
Is the environment variable empty? Or is it just not set?
Will the code hold if you set you own server to;
export MY_LSF_SERVER=""
b9dcb76
to
f8fe96f
Compare
andreas-el
reviewed
Sep 26, 2023
src/ert/config/queue_config.py
Outdated
Comment on lines
97
to
100
| f" {values[0]}. Check for an undefined environment variable. " | ||
| "The LSF_SERVER value is usually provided by default in a" | ||
| " site-configuration file. If you have no special needs you" | ||
| " can safely remove it from your configuration." |
There was a problem hiding this comment.
This is obviously up for discussion but you could consider something like this?
"Invalid server name specified for QUEUE_OPTION LSF LSF_SERVER:"
f" {values[0]}. Server name is currently an undefined environment variable. "
"The LSF_SERVER keyword is usually provided by the site-configuration file, "
"beware that you are effectively replacing the default value provided."
andreas-el
requested changes
Sep 26, 2023
larsevj
force-pushed
the
prevent_empty_LSF_SERVER_env
branch
from
f8fe96f
to
96d805f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
Resolves #6031
Pre review checklist
Pre merge checklist