Skip to content

Make the HITLOperator summary extension point public - #70391

Merged
potiuk merged 1 commit into
apache:mainfrom
shahar1:hitl-summary-extra-public
Aug 1, 2026
Merged

Make the HITLOperator summary extension point public#70391
potiuk merged 1 commit into
apache:mainfrom
shahar1:hitl-summary-extra-public

Conversation

@shahar1

@shahar1 shahar1 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

related: #70345

Human Summary

I had some concerns regarding _hitl_summary_extra being a private dictionary after I merged the above.
As the docs state that subclasses can extend HITLOperator, it should function better as a public attribute.

AI Summary

#70345 turned hitl_summary into a computed property (so templated subject/body are read after rendering) and moved runtime/subclass additions into a private _hitl_summary_extra dict. That left HITLOperator subclasses without a supported way to extend the summary — the pattern the old attribute explicitly documented ("subclasses can extend it") now silently mutates a throwaway snapshot — and it leaks the private attribute name into OpenLineage include_full_task_info events (which serialize the operator's __dict__, where a property never appears).

This renames the dict to a public hitl_summary_extra and documents the contract on the property: each access builds a fresh snapshot, mutation of the returned dict has no effect, and extensions go through hitl_summary_extra. Since #70345 has not shipped in a provider release yet, the rename is free of compatibility cost — waiting until after the next provider wave would turn it into a real break.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Fable 5)

Generated-by: Claude Code (Fable 5) following the guidelines

apache#70345 turned hitl_summary into a computed property and moved runtime
and subclass additions into a private _hitl_summary_extra dict. That
left HITLOperator subclasses without a supported way to extend the
summary (the pattern the old attribute explicitly documented) and leaks
the private attribute name into OpenLineage include_full_task_info
events. The property change has not shipped in a provider release yet,
so renaming the dict to a public hitl_summary_extra establishes the
replacement contract while it is still free of any compatibility cost.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the concern that prompted this. Once hitl_summary became a property, each access builds a fresh dict, so the documented "subclasses can extend it" pattern silently stopped working — anything written to the returned dict lands in a throwaway snapshot. Leaving the only working route behind a private name wasn't a real extension point.

The docstring addition is the part I value most here: writing down that the snapshot is rebuilt per access is what stops the next person rediscovering this the hard way.

Checked that the rename is complete — all ten internal sites, and nothing outside hitl.py referenced the private name. No compat exposure either, since _hitl_summary_extra was private and hasn't appeared in a released changelog entry.

One thought for later, not for this PR: a public mutable dict means subclasses have to call super().__init__() before touching it or they get an AttributeError. Both current subclasses do. An overridable hook would be order-independent, but the dict matches how the internals already use it and is the smaller change.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit 5efa9a1 into apache:main Aug 1, 2026
155 checks passed
@shahar1
shahar1 deleted the hitl-summary-extra-public branch August 1, 2026 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants