Repository navigation
[Python] Generalize recursive imports handling beyond jobs - #7167
Open
Sankalp-Mittal wants to merge 1 commit into
Open
Sankalp-Mittal wants to merge 1 commit into
Sankalp-Mittal wants to merge 1 commit into
Conversation
Detect dataclass cycles with a DFS from the resource root and quote the back-references, instead of hardcoding jobs.ForEachTask. Emit the _resolve_recursive_imports patch in any namespace that has quoted references, instead of only for jobs. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: ad29193
Top 3 slowest tests (at least 2 minutes):
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This generalizes the hack we used for generating the code for Jobs, to any resource that has circular imports, so we can remove the hardcoded references to
Jobs.Changes
quote_recursive_referencesnow finds cycles between dataclasses with a DFS from the resource root, and quotes each reference that points back to a dataclass still being visited. Previouslyjobs.ForEachTaskwas hardcoded._write_exportsnow emits_resolve_recursive_imports()for every dataclass with quoted references, in any namespace. This replaces theif namespace == "jobs"FIXME branch. The resolver buildsglobalnsfrom the classes that__init__.pyalready imports.Why
Resolves DECO-28419. Until now only jobs (
Task -> ForEachTask -> Task) could have recursive types. A new resource with a cycle would hit a circular import at load time, or fail to resolve forward references in_transform.Starting the DFS from the root breaks the edge that points back towards the root, so jobs keeps the same quoting as before (
ForEachTask.task). Generated_modelsare unchanged. The only generated diff is the jobs resolver: it no longer re-importsTask, and itsglobalnsdropsVariableOr(which isn't a string annotation, so it was never needed) and addsTaskParam.Tests
_modelsfile is byte-identical.codegen_tests/test_generated_dataclass_patch.pycovers a cycle outside jobs that goes through a list (Tree -> Node -> Branch -> list[Node]).Job.from_dictwith a nestedfor_each_taskbuilds a realTaskon Python 3.10–3.13../task pydabs-codegenend to end locally because of PyPI network failures. I ran the generator and ruff by hand instead, and I'm relying on CI for the codegen pytest suite.This pull request and its description were written by Isaac.