Skip to content
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

Remove provide_session decorator from TaskInstancePydantic methods #37853

Merged

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Mar 3, 2024

If we decorate these methods then the worker will try to create a session. But (I think...) there's no reason to do this. Sessions should be created on the static methods invoked by the API server right?

If we decorate these methods then the worker will try to create a session.  But there's no reason to do this.  Sessions should be created on the static methods invoked by the API server right?
@potiuk
Copy link
Member

potiuk commented Mar 3, 2024

Correct. Those should be removed.

@potiuk potiuk merged commit bb89d48 into apache:main Mar 3, 2024
56 of 57 checks passed
abhishekbhakat pushed a commit to abhishekbhakat/my_airflow that referenced this pull request Mar 5, 2024
…pache#37853)

If we decorate these methods then the worker will try to create a session.  But there's no reason to do this.  Sessions should be created on the static methods invoked by the API server right?
@utkarsharma2 utkarsharma2 added this to the Airflow 2.8.3 milestone Mar 6, 2024
@utkarsharma2 utkarsharma2 added the type:improvement Changelog: Improvements label Mar 6, 2024
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
…pache#37853)

If we decorate these methods then the worker will try to create a session.  But there's no reason to do this.  Sessions should be created on the static methods invoked by the API server right?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants