Fix template-field validation timing in ssh provider operators - #70315
Conversation
shahar1
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
- Please fix static checks.
- Make sure to remove the operator from the txt file.
ec71dfd to
627f053
Compare
|
Thanks for the review! Fixed the static check and removed both ssh entries from the exemption file. Let me know if anything else is needed. |
|
@bramhanandlingala noted an issue just before merging, please address this. Thank you! (please note that I merged |
# Conflicts: # scripts/ci/prek/validate_operators_init_exemptions.txt
|
Thanks @shahar1! Fixed — switched to if self.remote_host: as suggested, and added tests covering both the init no-mutation case and the execute() templated-value case. Ready for re-review. |
Please note that static checks currently fail |
|
Thanks for flagging, @shahar1 ! Dug into it — the failure wasn't related to the SSH changes at all. It was a stale exemption entry: SnowparkContainerJobOperator had already been fixed and its exemption removed on main, but a stale copy of that entry survived in our branch's validate_operators_init_exemptions.txt through a few merge conflict resolutions. Removed it and pushed — all checks are passing now. Ready for re-review whenever you have a chance! |
Fixes the
sshprovider's two entries from the #70296 exemption-list burn-down:SSHOperatorandSSHRemoteJobOperatorapplied logic to template fields inside__init__, before Jinja templating runs.SSHRemoteJobOperator:remote_base_dirwas validated in__init__, against the raw un-rendered template string. Moved validation toexecute().SSHOperator: when a pre-builtssh_hookwas injected,__init__pushed the un-renderedremote_hostonto it. Moved this toexecute().Both
__init__methods now only do plain assignments; the field-dependent logic runs inexecute(), after templating.Tests added for both operators covering the fixed behavior.
Related to #70296
Gen-AI disclosure: I used a generative AI tool to help identify the root
cause, write tests, and draft the PR description. I reviewed, tested, and
verified all changes locally before submitting.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude following the guidelines