fix(compliance-checks): don't require __file__ at module load on serverless - #795
surojitchowdhury wants to merge 3 commits into
Conversation
…erless
The compliance-checks workflow is submitted as a spark_python_task that runs
on Databricks serverless, where the entry script is executed via
exec(compile(...)) in a namespace with no __file__ bound. Evaluating
Path(__file__) at module import time raised NameError before main() ever ran,
so the scheduled compliance job died on startup every time on serverless.
Guard the sys.path bootstrap with globals().get("__file__"): use the
__file__-derived source root when present (unchanged local behaviour), and fall
back to the working directory (the deployed workflow folder on serverless) when
absent. The whole bootstrap is wrapped in try/except so it can never abort
module load.
Adds a smoke test that execs the module source with no __file__ in globals
(faithfully simulating serverless) and asserts no NameError, plus a case
proving normal import with __file__ present still works.
Fixes #685
Co-authored-by: Isaac
…erless Address cross-review of #685: - Serverless (no __file__): do NOT fabricate a sys.path entry. There is no reliable signal to derive the real source root -- the deployer uploads only the workflow folder (not the src tree) and serverless job environments cannot carry env vars ("compute.Environment doesn't support env_vars directly"). A cwd-based guess could prepend an unrelated dir and mask similarly named packages. The module's app imports are lazy (inside functions), so module import needs no sys.path entry; runtime from-src.* resolution relies on the environment/PYTHONPATH/installed package. If insufficient it fails later with a clear ImportError, not a cryptic NameError at load. - __file__ present (local/normal cluster): behaviour unchanged -- source root is Path(__file__).parent.parent.parent, prepended to sys.path. - Narrowed the exception handling: only the __file__-branch insert is guarded, and it warns to stderr instead of silently swallowing, so a real bootstrap failure is diagnosable while module import can never crash. Strengthen the smoke test to close the false-PASS gap: the serverless simulation now chdirs into a deployed-workflow-shaped temp dir and asserts sys.path is unchanged (no fabricated/cwd-derived entry), and the __file__ case asserts the correct source root is prepended. sys.path is snapshotted/restored per test; dependency stubs cannot mask a wrong entry because assertions inspect sys.path directly. Fixes #685 Co-authored-by: Isaac
Co-authored-by: Isaac
mvkonchits-db
left a comment
There was a problem hiding this comment.
Reviewed with the code-review skill. The fix correctly removes the import-time NameError from referencing __file__ at module load, but I don't think it makes the workflow actually runnable on serverless — leaving as a comment rather than approving so it can be verified.
-
The crash may just move later. On serverless the bootstrap now inserts nothing on
sys.path. Module load then succeeds, butmain()later hits lazy imports likefrom src.controller.compliance_manager import ComplianceManagerandfrom src.db_models.compliance import CompliancePolicyDb. The serverless env spec installs onlydatabricks-sdk/sqlalchemy/psycopg2-binary(not the ontos backend), so unless the runtime already has the backendsrcparent onPYTHONPATH, those raiseModuleNotFoundError— the same job failure, just later. -
Possible off-by-one in the non-serverless path. The insert uses
Path(__file__).parent.parent.parent=src/backend/src(thesrcpackage dir itself). Butfrom src.*resolves only whensrc/backend(the package's parent) is onsys.path. Inserting the package dir makesimport srcunresolvable in any env that relies solely on this insert. -
Tests don't cover the failure point. Both new tests set
__name__ != '__main__', somain()never runs and the lazyfrom src.*imports are never exercised — the suite passes without touching the thing that actually fails on serverless. Alsoassert sys.path == sys_path_beforeis order-dependent and can flake if a real dep import mutatessys.pathfirst.
Suggestion: validate against a real serverless run (or a test that invokes main()/the lazy imports under a simulated serverless sys.path) before merging, and double-check the insert targets src/backend, not src/backend/src.
Fixes #685
Summary (plain language)
Ontos runs its compliance checks as a scheduled Databricks job on serverless compute. The entry script tried, at its very first executable line, to work out its own folder location using Python's
__file__variable. On serverless the runtime executes the script viaexec(compile(...))in a namespace where__file__is not bound, so evaluatingPath(__file__)raisedNameErrorat module load — beforemain()ever ran. The scheduled compliance run therefore died on startup every time on serverless.This change makes the module-level
sys.pathbootstrap no longer depend on__file__being present, so the job starts on serverless, while behaving exactly as before on a normal cluster / locally.Change
src/backend/src/workflows/compliance_checks/compliance_checks.py— the single unguarded lineis replaced with a guarded bootstrap:
Approach
A key fact drives the design: the workflow's
from src.*imports are lazy (insideload_policies/run_policy/main), never at module top level. So importing this module never itself needssrconsys.path— the entry only matters at runtime on a normal cluster / locally.__file__present (local dev / normal cluster): behaviour is unchanged — the backend source root isPath(__file__).parent.parent.parent, prepended tosys.path.__file__(serverlessexec(compile(...))): insert nothing. There is no reliable signal to derive the real source root here — the deployer uploads only this workflow folder (not thesrctree), and serverless job environments cannot carry env vars. Guessing from the current working directory could prepend an unrelated directory and mask similarly named packages, so the bootstrap adds nothing and lets the runtime environment (installed package /PYTHONPATH) resolve the lazy imports. If that is ever insufficient, the failure surfaces later as a clearImportErrorinstead of a crypticNameErrorat module load.__file__-present insert is narrowly guarded: an unexpected failure is reported tostderr(diagnosable) but can never abort module import.Scope is strictly the path-bootstrap; no unrelated logic was refactored, and nothing under
.github/workflows/is touched.Tests
Adds
src/backend/tests/test_compliance_checks_workflow.pywith two smoke tests that go beyond "no crash" and assert path correctness:execs it in a globals dict with no__file__({"__name__": "not_main"}), afterchdir-ing into a simulated deployed workflow folder. Asserts: noNameError;main()did not run; and — crucially —sys.pathis left exactly unchanged (nothing prepended), with the cwd-derived grandparent specifically absent. This closes the false-PASS gap where a wrong path would otherwise slip through.execs with__file__present and asserts the correct source root is inserted atsys.path[0].Top-level third-party imports (
sqlalchemy,databricks.sdk) are stubbed when not importable, so the tests isolate the__file__bootstrap; because the assertions inspectsys.pathdirectly, stubbing cannot mask an incorrect entry.Gate output
Only 2 files changed (the workflow script + the new test); nothing under
.github/workflows/.