Experiment: resource dependency graph as YAML during build (alpha flag) - #3050
Open
ronpal wants to merge 5 commits into
Open
Experiment: resource dependency graph as YAML during build (alpha flag)#3050ronpal wants to merge 5 commits into
ronpal wants to merge 5 commits into
Conversation
Export the instance-level resource dependency graph (topological order, per-module edges, missing refs, and cycles) as YAML after build.
Contributor
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
Contributor
Author
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to export instance-level resource dependency graphs as YAML during the build process, controlled by a feature flag. Feedback highlights a style guide violation regarding the use of untyped dictionaries instead of Pydantic models, as well as a bug in the cycle-breaking logic that entirely omits cyclic nodes from the topological order.
Comment on lines
+18
to
+117
| Node = tuple[type[ResourceIO], Identifier] | ||
|
|
||
|
|
||
| def _node_dict(crud_cls: type[ResourceIO], identifier: Identifier) -> dict[str, Any]: | ||
| return {"type": crud_cls.kind, "id": str(identifier)} | ||
|
|
||
|
|
||
| def _sort_key(node: Node) -> tuple[str, str]: | ||
| crud_cls, identifier = node | ||
| return crud_cls.kind, str(identifier) | ||
|
|
||
|
|
||
| def _topological_order(graph: dict[Node, set[Node]], built_nodes: set[Node]) -> tuple[list[Node], list[list[Node]]]: | ||
| """Topologically sort the built resources. | ||
|
|
||
| Returns the ordered built nodes and any detected cycles. Cycles are broken so that a | ||
| partial ordering can still be produced (mirrors the view-sorting behavior elsewhere). | ||
| """ | ||
| cycles: list[list[Node]] = [] | ||
| working = {node: set(deps) for node, deps in graph.items()} | ||
| while True: | ||
| try: | ||
| ordered = list(TopologicalSorter(working).static_order()) | ||
| break | ||
| except CycleError as e: | ||
| cycle_nodes: list[Node] = list(e.args[1]) | ||
| cycles.append(cycle_nodes) | ||
| cycle_set = set(cycle_nodes) | ||
| working = {n: (d - cycle_set) for n, d in working.items() if n not in cycle_set} | ||
| return [node for node in ordered if node in built_nodes], cycles | ||
|
|
||
|
|
||
| def build_dependency_graph(build_folder: BuildFolder) -> dict[str, Any]: | ||
| """Build the serializable instance-level dependency graph for a built folder.""" | ||
| built_nodes: set[Node] = { | ||
| (resource.crud_cls, resource.identifier) | ||
| for module in build_folder.built_modules | ||
| for resource in module.resources | ||
| } | ||
|
|
||
| graph: dict[Node, set[Node]] = {} | ||
| for module in build_folder.built_modules: | ||
| for resource in module.resources: | ||
| node: Node = (resource.crud_cls, resource.identifier) | ||
| graph.setdefault(node, set()).update(resource.dependencies) | ||
|
|
||
| ordered, cycles = _topological_order(graph, built_nodes) | ||
|
|
||
| modules_output: list[dict[str, Any]] = [] | ||
| for module in build_folder.built_modules: | ||
| if not module.resources: | ||
| continue | ||
| resources_output: list[dict[str, Any]] = [] | ||
| for resource in sorted(module.resources, key=lambda r: _sort_key((r.crud_cls, r.identifier))): | ||
| entry: dict[str, Any] = { | ||
| **_node_dict(resource.crud_cls, resource.identifier), | ||
| "source": relative_to_if_possible(resource.source_path).as_posix(), | ||
| } | ||
| depends_on = _depends_on(resource, built_nodes) | ||
| if depends_on: | ||
| entry["depends_on"] = depends_on | ||
| resources_output.append(entry) | ||
| modules_output.append( | ||
| { | ||
| "module": str(module.module_id), | ||
| "resources": resources_output, | ||
| } | ||
| ) | ||
|
|
||
| output: dict[str, Any] = { | ||
| "topological_order": [ | ||
| { | ||
| **_node_dict(crud_cls, identifier), | ||
| } | ||
| for crud_cls, identifier in ordered | ||
| ], | ||
| "modules": modules_output, | ||
| } | ||
| if cycles: | ||
| output["cycles"] = [[_node_dict(crud_cls, identifier) for crud_cls, identifier in cycle] for cycle in cycles] | ||
| return output | ||
|
|
||
|
|
||
| def _depends_on(resource: BuiltResource, built_nodes: set[Node]) -> list[dict[str, Any]]: | ||
| depends_on: list[dict[str, Any]] = [] | ||
| for crud_cls, identifier in sorted(resource.dependencies, key=_sort_key): | ||
| dependency = _node_dict(crud_cls, identifier) | ||
| if (crud_cls, identifier) not in built_nodes: | ||
| dependency["missing"] = True | ||
| depends_on.append(dependency) | ||
| return depends_on | ||
|
|
||
|
|
||
| def write_dependency_graph(build_folder: BuildFolder, output_path: Path) -> None: | ||
| """Serialize the instance-level dependency graph to a YAML file.""" | ||
| graph = build_dependency_graph(build_folder) | ||
| safe_write(output_path, yaml_safe_dump(graph, sort_keys=False)) | ||
|
|
||
|
|
||
| __all__ = ["build_dependency_graph", "write_dependency_graph"] |
Contributor
There was a problem hiding this comment.
Issues Identified:
- Style Guide Violation (Type Safety & Strong Typing): The current implementation uses untyped dictionaries (
dict[str, Any]) for representing the dependency graph nodes, modules, and the graph itself. The Cognite Python Style Guide explicitly requires using Pydantic models or dataclasses instead of untyped dictionaries to ensure type safety and strong typing. - Bug in Cycle-Breaking Logic: In
_topological_order, when a cycle is detected, the nodes incycle_setare completely removed from theworkingdictionary (if n not in cycle_set). This means any resource involved in a cycle will be entirely omitted from the finaltopological_orderlist in the exported YAML, even though they are built resources that need to be deployed.
Solution:
We can resolve both issues by:
- Defining Pydantic models (
ResourceNode,DependencyNode,ModuleResourceNode,ModuleDependency,DependencyGraph) to represent the graph structure. - Modifying the cycle-breaking logic to keep the nodes in the graph but break the cycle by removing only the dependencies between the cycle nodes (
working = {n: (d - cycle_set) if n in cycle_set else d for n, d in working.items()}).
from pydantic import BaseModel, Field
Node = tuple[type[ResourceIO], Identifier]
class ResourceNode(BaseModel):
type: str
id: str
class DependencyNode(ResourceNode):
missing: bool | None = None
class ModuleResourceNode(ResourceNode):
source: str
depends_on: list[DependencyNode] | None = None
class ModuleDependency(BaseModel):
module: str
resources: list[ModuleResourceNode]
class DependencyGraph(BaseModel):
topological_order: list[ResourceNode]
modules: list[ModuleDependency]
cycles: list[list[ResourceNode]] | None = None
def _sort_key(node: Node) -> tuple[str, str]:
crud_cls, identifier = node
return crud_cls.kind, str(identifier)
def _topological_order(graph: dict[Node, set[Node]], built_nodes: set[Node]) -> tuple[list[Node], list[list[Node]]]:
"""Topologically sort the built resources.
Returns the ordered built nodes and any detected cycles. Cycles are broken so that a
partial ordering can still be produced (mirrors the view-sorting behavior elsewhere).
"""
cycles: list[list[Node]] = []
working = {node: set(deps) for node, deps in graph.items()}
while True:
try:
ordered = list(TopologicalSorter(working).static_order())
break
except CycleError as e:
cycle_nodes: list[Node] = list(e.args[1])
cycles.append(cycle_nodes)
cycle_set = set(cycle_nodes)
working = {n: (d - cycle_set) if n in cycle_set else d for n, d in working.items()}
return [node for node in ordered if node in built_nodes], cycles
def build_dependency_graph(build_folder: BuildFolder) -> DependencyGraph:
"""Build the serializable instance-level dependency graph for a built folder."""
built_nodes: set[Node] = {
(resource.crud_cls, resource.identifier)
for module in build_folder.built_modules
for resource in module.resources
}
graph: dict[Node, set[Node]] = {}
for module in build_folder.built_modules:
for resource in module.resources:
node: Node = (resource.crud_cls, resource.identifier)
graph.setdefault(node, set()).update(resource.dependencies)
ordered, cycles = _topological_order(graph, built_nodes)
modules_output: list[ModuleDependency] = []
for module in build_folder.built_modules:
if not module.resources:
continue
resources_output: list[ModuleResourceNode] = []
for resource in sorted(module.resources, key=lambda r: _sort_key((r.crud_cls, r.identifier))):
depends_on = _depends_on(resource, built_nodes)
resources_output.append(
ModuleResourceNode(
type=resource.crud_cls.kind,
id=str(resource.identifier),
source=relative_to_if_possible(resource.source_path).as_posix(),
depends_on=depends_on or None,
)
)
modules_output.append(
ModuleDependency(
module=str(module.module_id),
resources=resources_output,
)
)
output = DependencyGraph(
topological_order=[
ResourceNode(type=crud_cls.kind, id=str(identifier))
for crud_cls, identifier in ordered
],
modules=modules_output,
)
if cycles:
output.cycles = [
[ResourceNode(type=crud_cls.kind, id=str(identifier)) for crud_cls, identifier in cycle]
for cycle in cycles
]
return output
def _depends_on(resource: BuiltResource, built_nodes: set[Node]) -> list[DependencyNode]:
depends_on: list[DependencyNode] = []
for crud_cls, identifier in sorted(resource.dependencies, key=_sort_key):
is_missing = (crud_cls, identifier) not in built_nodes
depends_on.append(
DependencyNode(
type=crud_cls.kind,
id=str(identifier),
missing=True if is_missing else None,
)
)
return depends_on
def write_dependency_graph(build_folder: BuildFolder, output_path: Path) -> None:
"""Serialize the instance-level dependency graph to a YAML file."""
graph = build_dependency_graph(build_folder)
safe_write(output_path, yaml_safe_dump(graph.model_dump(exclude_none=True), sort_keys=False))
__all__ = ["build_dependency_graph", "write_dependency_graph"]References
- Use dataclasses and Pydantic models for complex data structures instead of untyped dictionaries (dict[str, Any]). (link)
☂️ Code Coverage
Overall Coverage
New Files
Modified Files
|
ronpal
marked this pull request as ready for review
June 3, 2026 14:02
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3050 +/- ##
==========================================
- Coverage 85.93% 85.85% -0.08%
==========================================
Files 474 475 +1
Lines 43567 43683 +116
==========================================
+ Hits 37440 37506 +66
- Misses 6127 6177 +50
🚀 New features to boost your workflow:
|
Use TOPOLOGY / topology in cdf.toml and --topology on cdf build.
ronpal
changed the title
…at/build-dependency-graph-export
…uilds - Add print_dependency_graph_json() to output the dependency graph as JSON to stdout instead of writing YAML to a file - Skip the module/CLI version mismatch check when __version__ is 0.0.0 (local development builds)
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
1 participant
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.
Description
Adds an alpha-gated
--dependency-graphoption tocdf buildthat writes the instance-level resource dependency graph as YAML after a successful build.The output includes:
topological_order— built resources in dependency order (cycles broken and listed undercycleswhen present)modules— per-module resources withdepends_onedges; references outside the build are markedmissing: trueEnable in
cdf.toml:Example:
Bump
Changelog
Added
DEPENDENCY_GRAPHand--dependency-graphoncdf buildto export the resource dependency graph as YAML.