Pass existing environment variables to agentless flow runs #4917
Conversation
|
@TylerWanner @anticorrelator review me please |
|
LGTM! Feels like a usability lift, though I wouldn't suggest anyone to inject env vars from execution environment into a flow run in production, if avoidable. It is good practice to make the execution environment as agnostic as possible, and it is a bad idea to throw some undefined sack of env vars around like this in production, but this can be very useful in development. |
|
I see now this is actually True by default and has no interface for toggling. I would prefer it be False by default, but might be convinced otherwise. Would strongly prefer that this add a flag to toggle either way. |
|
Sorry for waffling here. Given that agentless execution is a subprocess and not directly equivalent to running via agent, it does make sense to include the host env vars by default. In fact, this expectation should be clear enough that I do not think it needs a toggle. Anybody using --execute is effectively taking ownership of their execution environment (away from Prefect), and should expect it to work this way. |
|
|
||
| # Should pass the correct flow run id to wait for | ||
| mocks.wait_for_flow_run_start_time.assert_called_once_with("flow-run-id") | ||
|
|
||
| # Merge the starting env and the env generated for a flow run | ||
| base_env = os.environ.copy() if include_local_env else {} |
There was a problem hiding this comment.
I've always had an idiosyncratic preference for dict() instead of {} because of the visual similarity to set syntax
There was a problem hiding this comment.
That makes sense, I may do that more in the future.
Summary
Includes the local environment in agentless flow runs by default. This was previously not available as I did not want to expose the entire local environment to flow runs--it seemed like a security risk.
Alternatively, we could allow opt-in for a list of environment variables or encourage users to pass thing via prefect local secrets.
Importance
Users frequently store environment variables that their flows needs on the machine their flow runs on
Checklist
This PR:
changes/directory (if appropriate)docs/outline.tomlfor API reference docs (if appropriate)