Skip to content

Commit

Permalink
Stop disabling declaring in _create_subnode
Browse files Browse the repository at this point in the history
_create_subnode temporarily disables _context.declaring to avoid
triggering the altered `__getattribute__` behaviour in
TemplateInvocatorSubNodeMixin in two situations:

 - Pydantic's implementation of `!=`, which is attempting to read field
   values, and will raise a validation error due to the altered field
   names being of incorrect type.
 - Accessing `__class__` on a Pydantic subnode object with no build_obj
   set.

Instead, fix this by:

 - Using `is not` instead of `!=`, which is correct where we use it,
   and also faster.
 - Fixing `__getattribute__` to no longer fail if used on an object with
   no build_obj set.

Signed-off-by: Alice Purcell <[email protected]>
  • Loading branch information
alicederyn committed Sep 4, 2024
1 parent b1933d8 commit 4d15b67
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
4 changes: 1 addition & 3 deletions src/hera/workflows/_meta_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,8 +539,7 @@ def _create_subnode(
assert _context.pieces

template_ref = None
_context.declaring = False
if _context.pieces[0] != self and isinstance(self, WorkflowTemplate):
if _context.pieces[0] is not self and isinstance(self, WorkflowTemplate):
# Using None for cluster_scope means it won't appear in the YAML spec (saving some bytes),
# as cluster_scope=False is the default value
template_ref = TemplateRef(
Expand Down Expand Up @@ -571,7 +570,6 @@ def _create_subnode(
_context.pieces[-1]._current_task_depends.clear()

subnode._build_obj = HeraBuildObj(subnode._subtype, output_class)
_context.declaring = True
return subnode

@_add_type_hints(Script) # type: ignore
Expand Down
6 changes: 4 additions & 2 deletions src/hera/workflows/_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,11 +715,13 @@ class TemplateInvocatorSubNodeMixin(BaseMixin):
_build_obj: Optional[HeraBuildObj] = PrivateAttr(None)

def __getattribute__(self, name: str) -> Any:
if _context.declaring:
try:
# Use object's __getattribute__ to avoid infinite recursion
build_obj = object.__getattribute__(self, "_build_obj")
assert build_obj # Assertions to fix type checking
except AttributeError:
build_obj = None

if build_obj and _context.declaring:
fields = get_fields(build_obj.output_class)
annotations = get_field_annotations(build_obj.output_class)
if name in fields:
Expand Down

0 comments on commit 4d15b67

Please sign in to comment.