Avoid direct use of os.environ #71
Conversation
|
@webknjaz @tadeboro I need to pick your brain on one thing. Both Runtime() and AnsibleConfig() to need access to our environ and both execute code using this environment. I could try to pass Runtime.environ to AnsibleConfig in the constructor. Still this will not enable me to reuse Runtime.exec() inside config instance. Another option would be to pass runtime instance when creating AnsibleConfig, but this will mean that those two classes will cross linked. I am not sure how big of a problem is that but at least it allows us to keep execution in one place, which would clearly be a big advantage towards enabling use of ansible-runner. I already considered keeping environ only inside config, but that is not sorting the execution bit, only making a little bit harder to access via WDYT? |
From now on, Runtime() will make a copy of os.environ and use that instead, eventually making required modifications to it. This ensures that modifications made by our runtime will not affect current interpreter environment. This sorts a bug observed during testing where modification to os.environ started to cross-contaminate between tests. Another benefit from this change is that it makes easier to adopt use of containerized/remote ansible runtimes like execution engine. This change should be considered major because it means that any consumer of the library must use of `Runtime.exec()` to execute commands using the modified environment. If they used subprocess.run or similar they will need at least to pass Runtime.environ in order to avoid execution errors as os.environ is no longer altered.
I would go in a slightly different direction and make |
From now on, Runtime() will make a copy of os.environ and use that
instead, eventually making required modifications to it. This ensures
that modifications made by our runtime will not affect current
interpreter environment.
This sorts a bug observed during testing where modification to
os.environ started to cross-contaminate between tests.
Another benefit from this change is that it makes easier to adopt use
of containerized/remote ansible runtimes like execution engine. This
change should be considered major because it means that any consumer
of the library must use of
Runtime.exec()to execute commandsusing the modified environment. If they used subprocess.run or similar
they will need at least to pass Runtime.environ in order to avoid
execution errors as os.environ is no longer altered.