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

Resolving shell environment variables name space conflicts with parm #28

Closed
wants to merge 17 commits into from
Closed

Resolving shell environment variables name space conflicts with parm #28

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
26eb81d
excludes updating config dic with values that are in enviroment and i…
TerryMcGuinness-NOAA May 22, 2024
90b2d79
added bash as shell when geting config env
TerryMcGuinness-NOAA May 31, 2024
2e91edc
missed comma after shell in sub
TerryMcGuinness-NOAA May 31, 2024
bbeefc3
made sure bash can be anywere for subprocess in configure getting con…
TerryMcGuinness-NOAA May 31, 2024
b918dc4
Update src/wxflow/configuration.py
aerorahul Jun 5, 2024
9d874ac
unskip in runner for configuration tests
TerryMcGuinness-NOAA Jun 5, 2024
98a8ef0
removed skip configuration tests and used new method that checks for …
TerryMcGuinness-NOAA Jun 5, 2024
8e4b3e1
resolved conflicts
TerryMcGuinness-NOAA Jun 5, 2024
07aa4c0
restore correct assertion in config1 parse test
TerryMcGuinness-NOAA Jun 5, 2024
404cf0a
added new solution that checkes in the scripts for name clashes and t…
TerryMcGuinness-NOAA Jun 6, 2024
64d73f8
added the key search to include = so USER was not picked up
TerryMcGuinness-NOAA Jun 6, 2024
9740026
removed Optoinal from import list
TerryMcGuinness-NOAA Jun 6, 2024
b8dd227
changed order of imports in configureation file due to unkown pynorms…
TerryMcGuinness-NOAA Jun 6, 2024
86e31db
added dynamic regex with export
TerryMcGuinness-NOAA Jun 11, 2024
57af2f5
updated regex in config
TerryMcGuinness-NOAA Jun 25, 2024
761bcdf
Merge branch 'NOAA-EMC:develop' into config_env_xor
TerrenceMcGuinness-NOAA Jun 26, 2024
5382a0e
simplfied version of _get_script_env classmethond in Configuration Class
TerryMcGuinness-NOAA Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/wxflow/configuration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import glob
import os
import re
import random
import shutil
import subprocess
from pathlib import Path
from pprint import pprint
Expand Down Expand Up @@ -96,7 +98,12 @@ def print_config(self, files: Union[str, bytes, list]) -> None:
def _get_script_env(cls, scripts: List) -> Dict[str, Any]:
default_env = cls._get_shell_env([])
and_script_env = cls._get_shell_env(scripts)
vars_just_in_script = set(and_script_env) - set(default_env)
keys_in_scripts = set()
regex_pattern = 'export\\s+\\b(' + '|'.join(map(re.escape, default_env.keys())) + ')(?==)'
for script in scripts:
result = subprocess.run(['grep', '-o', '-P', regex_pattern, script], stdout=subprocess.PIPE)
keys_in_scripts.update(result.stdout.decode().split())
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be explained a bit?
The task is simple here; get a dictionary of key-value pairs set by a shell script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old approach was faulty, this one (not intended to be used) resorted to using a regular expression to fish out any environment variable that is in the user spaces that is also in a config file. It is here only to show what is needed to pass the unit tests. I don't think this is good solution, so I'm thinking of another approach entirely to avoid chicken-before-egg dilemma that this will not solve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and then just check if the key is in both!

vars_just_in_script = set(and_script_env) - set(default_env) | keys_in_scripts
union_env = dict(default_env)
union_env.update(and_script_env)
return dict([(v, union_env[v]) for v in vars_just_in_script])
Expand All @@ -107,8 +114,9 @@ def _get_shell_env(scripts: List) -> Dict[str, Any]:
runme = ''.join([f'source {s} ; ' for s in scripts])
magic = f'--- ENVIRONMENT BEGIN {random.randint(0,64**5)} ---'
runme += f'/bin/echo -n "{magic}" ; /usr/bin/env -0'
bash_path = shutil.which('bash')
aerorahul marked this conversation as resolved.
Show resolved Hide resolved
with open('/dev/null', 'w') as null:
env = subprocess.Popen(runme, shell=True, stdin=null.fileno(),
env = subprocess.Popen(runme, shell=True, executable=bash_path, stdin=null.fileno(),
stdout=subprocess.PIPE)
(out, err) = env.communicate()
out = out.decode()
Expand Down
3 changes: 0 additions & 3 deletions tests/test_configuration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ def test_configuration_config_dir(tmp_path, create_configs):
assert cfg.config_dir == tmp_path


@pytest.mark.skip(reason="fails in GH runner, passes on localhost")
def test_configuration_config_files(tmp_path, create_configs):
cfg = Configuration(tmp_path)
config_files = [str(tmp_path / 'config.file0'), str(tmp_path / 'config.file1')]
Expand All @@ -157,14 +156,12 @@ def test_find_config(tmp_path, create_configs):
assert str(tmp_path / 'config.file0') == file0


@pytest.mark.skip(reason="fails in GH runner, passes on localhost")
def test_parse_config1(tmp_path, create_configs):
cfg = Configuration(tmp_path)
f0 = cfg.parse_config('config.file0')
assert file0_dict == f0


@pytest.mark.skip(reason="fails in GH runner, passes on localhost")
def test_parse_config2(tmp_path, create_configs):
cfg = Configuration(tmp_path)
ff = cfg.parse_config(['config.file0', 'config.file1'])
Expand Down
Loading