fix(sandbox): suppress policy warnings during initial workflow load - #1690
fix(sandbox): suppress policy warnings during initial workflow load#1690Sakshamm-Goyal wants to merge 4 commits into
Conversation
|
Hey @Sakshamm-Goyal, thanks for taking a look at this! The fix isn't quite correct as the import time dependencies of the workflow module are intended to trigger the warnings/exceptions. Those dependencies are loaded repeatedly when workflows are executed in the sandbox and the warnings are intended to highlight that those dependencies might be worth explicitly passing through to share them across workflow instances and avoid the processing/memory overhead of reloading them. Addressing this issue would need to be scoped to suppressing the warnings for the imported workflow module only and allow the import time dependencies of that module to trigger configured the warnings or exceptions. |
|
Thanks — narrowed the suppression to the initial workflow module only in commit fb1b110. Import-time dependencies now continue through the configured notification policy, including both warning and raise modes. Added regression coverage for the workflow-module/dependency split; focused sandbox tests (4) plus Ruff and Pyright pass locally. CI is rerunning on the updated head. |
VegetarianOrc
left a comment
There was a problem hiding this comment.
The approach looks okay to me, but the new check should be more narrowly applied.
Also please update the PR description to reflect the new behavior and include a changelog entry describing the new behavior.
|
Addressed in 5788b9f. The initial-module condition now applies only inside the warning/raise notification block, so the normal importer flow is unchanged. I also updated the PR description and added the Unreleased changelog entry. Focused sandbox tests (4), Ruff, and Pyright remain green; CI is rerunning. |
5788b9f to
de6e055
Compare
|
The failed lint matrix was caused by Ruff I001 in the added fixture module, not the sandbox behavior. I reordered that module’s imports only and force-pushed the amended commit (de6e055). Local validation passed: Ruff import/full checks, Ruff format check, Pyright, and the two focused initial-import sandbox tests. Fresh CI is now running. |
|
Correction to the CI status: the new fork-PR workflows entered action_required with zero jobs, so they need upstream approval before the matrix can run. CODEOWNERS and CLA are already green. The local validation reported above remains clean; I will monitor the approved rerun rather than attempting to bypass this gate. |
Summary
Validation
Fixes #1254.