fix: inherit parent process env in Python discovery subprocess#10818
fix: inherit parent process env in Python discovery subprocess#10818IzaakGough wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Python runtime delegate to inherit the parent shell environment (process.env) when executing discovery subprocesses in modulesDir() and serveAdmin(), and adds corresponding unit tests. A review comment identifies a TypeScript compilation error in serveAdmin() where spreading process.env directly results in an incompatible type, and suggests casting it to Record<string, string> to resolve the issue.
There was a problem hiding this comment.
Thanks @IzaakGough, the approach is right and the tests are a good addition. Requesting changes for one issue: the debug log now dumps the entire parent shell env into firebase-debug.log (details inline).
There was a problem hiding this comment.
Could we potentially merge process.env here within runWithVirtualEnv instead of spreading it in every caller? It could reduce the redundant boilerplate and prevent existing and future callers from forgetting to include it.
There was a problem hiding this comment.
Really good catch, thanks! I will make these changes now
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
wandamora
left a comment
There was a problem hiding this comment.
LGTM for google review.
Fixes #7397
This PR ensures that the Python discovery sub-process inherits environment variables from the parent process rather than ignoring them.
Testing