Skip to content

Move template field validation out of __init__ for Papermill operator (#70296) - #70357

Closed
AmoghAtreya wants to merge 1 commit into
apache:mainfrom
AmoghAtreya:fix/70296-cohere-papermill-operator-init
Closed

Move template field validation out of __init__ for Papermill operator (#70296)#70357
AmoghAtreya wants to merge 1 commit into
apache:mainfrom
AmoghAtreya:fix/70296-cohere-papermill-operator-init

Conversation

@AmoghAtreya

@AmoghAtreya AmoghAtreya commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Refactored CohereEmbeddingOperator and PapermillOperator to ensure template fields are validated during execution or render time rather than inside __init__, aligning with Airflow operator standards.

  • Moved initialization checks dependent on template fields out of __init__.
  • Updated corresponding unit tests in Cohere and Papermill provider test suites.
  • Removed CohereEmbeddingOperator and PapermillOperator entries from scripts/ci/prek/validate_operators_init_exemptions.txt.

Related Issue

works on: #70296

Was generative AI tooling used to co-author this PR?
  • Yes (Gemini)

Generated-by: Gemini following the guidelines

@boring-cyborg

boring-cyborg Bot commented Jul 24, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@shahar1 shahar1 removed the backport-to-v3-3-test Backport to v3-3-test label Jul 24, 2026
@shahar1 shahar1 changed the title refactor: move template field validation out of __init__ for Cohere and Papermill operators (#70296) Move template field validation out of __init__ for Cohere and Papermill operators (#70296) Jul 24, 2026
shahar1
shahar1 reviewed Jul 24, 2026
View reviewed changes

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fix static checks

@AmoghAtreya
AmoghAtreya force-pushed the fix/70296-cohere-papermill-operator-init branch 2 times, most recently from 7491d37 to 0ab078f Compare July 24, 2026 08:37
@AmoghAtreya

AmoghAtreya commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Should be fixed now @shahar1

@AmoghAtreya

AmoghAtreya commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think the Mergeable check is struck and needs to be retriggered, is there any way you can retrigger just that one on the admin end or should I close and reopen the PR to re-run checks? Other than that, all checks are passing. @shahar1

@shahar1

shahar1 commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think the Mergeable check is struck and needs to be retriggered, is there any way you can retrigger just that one on the admin end or should I close and reopen the PR to re-run checks? Other than that, all checks are passing. @shahar1

Ignore this for now, the other checks are those that are important. I'll review soon.

shahar1
shahar1 reviewed Jul 24, 2026
View reviewed changes
Comment thread providers/cohere/tests/unit/cohere/operators/test_embedding.py
@AmoghAtreya

AmoghAtreya commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 please let me know if there's anything else you want me to fix!

@shahar1

shahar1 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@shahar1 please let me know if there's anything else you want me to fix!

It seems that you accidentally deleted existing tests from ‎providers/cohere/tests/unit/cohere/operators/test_embedding.py - please try to revert that

@AmoghAtreya @cursoragent
Move template-field validation out of PapermillOperator __init__
3db70c4 
Template fields are rendered after the constructor runs, so validating
input_nb and output_nb in __init__ acts on the un-rendered Jinja
expression. Defer the checks to execute() and drop the class from the
validate-operators-init exemption list.

Co-authored-by: Cursor <cursoragent@cursor.com>
@AmoghAtreya
AmoghAtreya force-pushed the fix/70296-cohere-papermill-operator-init branch from c484cb1 to 3db70c4 Compare July 26, 2026 06:19
@AmoghAtreya

AmoghAtreya commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@shahar1 please let me know if there's anything else you want me to fix!

It seems that you accidentally deleted existing tests from ‎providers/cohere/tests/unit/cohere/operators/test_embedding.py - please try to revert that

Sorry, just fixed that

@shahar1 shahar1 changed the title Move template field validation out of __init__ for Cohere and Papermill operators (#70296) Move template field validation out of __init__ for Papermill operator (#70296) Jul 26, 2026
@shahar1

shahar1 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Apparently #70435 was created first and already merged.
There are still some other operators to handle :)

@shahar1 shahar1 closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shahar1 shahar1 shahar1 left review comments
@potiuk potiuk Awaiting requested review from potiuk potiuk is a code owner
@ashb ashb Awaiting requested review from ashb ashb is a code owner
@gopidesupavan gopidesupavan Awaiting requested review from gopidesupavan gopidesupavan is a code owner
@amoghrajesh amoghrajesh Awaiting requested review from amoghrajesh amoghrajesh is a code owner
@jscheffl jscheffl Awaiting requested review from jscheffl jscheffl is a code owner
@bugraoz93 bugraoz93 Awaiting requested review from bugraoz93 bugraoz93 is a code owner
@jason810496 jason810496 Awaiting requested review from jason810496 jason810496 is a code owner

Assignees

No one assigned

Labels

area:dev-tools area:providers provider:cohere provider:papermill

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AmoghAtreya @shahar1