-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deferrable mode for CreateBatchPredictionJobOperator #37818
Deferrable mode for CreateBatchPredictionJobOperator #37818
Conversation
b1d38bf
to
8efd4b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactoring @moiseenkov, async operators are better as those are non-blocking and don't hold up workers while waiting for a response.
You could improve the tests like I did with my async msgraph operator here, so that you simulate the whole async execution cycle including the trigger execution. You'll see that I've create an execute_operator method, that way we simulate the whole behaviour in the same main thread and then we can do more than just simple verifications, we can actually also assert the result of that execution. Maybe we could move that execute_operator helper method somewhere to some central helper method in the tests package so it can be re-used across different tests for async operators. Doing so also allows you, like I did, to accumulate XCom interactions with the context. Don't know what the other Airflow project maintainers think of this approach.
Hi @eladkal @potiuk @pankajastro @hussein-awala ! |
The apache#37818 bumped requirement for google-cloud-aiplatform to a version that requires Pydantic so we need to remove it when we are running "no-pydantic" tests. The tests that import it should be skipped when the dependency is removed.
Deferrable mode for the
CreateBatchPredictionJobOperator
is introduced.