Skip to content

Add werkzeug package to sendgrid#48976

Merged
gopidesupavan merged 1 commit intoapache:mainfrom
gopidesupavan:add-werkzeug-to-sendgrid
Apr 8, 2025
Merged

Add werkzeug package to sendgrid#48976
gopidesupavan merged 1 commit intoapache:mainfrom
gopidesupavan:add-werkzeug-to-sendgrid

Conversation

@gopidesupavan
Copy link
Copy Markdown
Member

Looks like werkzeug is required for the sendgrid. failing Low dep tests

https://github.com/apache/airflow/actions/runs/14342959080/job/40206921048?pr=48819#step:6:2431


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@gopidesupavan
Copy link
Copy Markdown
Member Author

Looks like previously it worked it has package from the core, now its removed from core a052453 , i think its okay to add in sendrig?

@gopidesupavan gopidesupavan merged commit 4d5c2e3 into apache:main Apr 8, 2025
49 checks passed
@gopidesupavan gopidesupavan deleted the add-werkzeug-to-sendgrid branch April 8, 2025 21:42
@eladkal
Copy link
Copy Markdown
Contributor

eladkal commented Apr 10, 2025

The provider doesn't have code that requires werkzeug, which means that it's something comping from upstream
so we need upstream to fix missing dependency they forgot to set. Is there an issue to track?

@gopidesupavan
Copy link
Copy Markdown
Member Author

The provider doesn't have code that requires werkzeug, which means that it's something comping from upstream so we need upstream to fix missing dependency they forgot to set. Is there an issue to track?

Not created i have missed that, i think the tests are failing for lower deps , probably we can add it optional deps group and remove from the dependencies, WDYT?

@eladkal
Copy link
Copy Markdown
Contributor

eladkal commented Apr 10, 2025

Not created i have missed that, i think the tests are failing for lower deps , probably we can add it optional deps group and remove from the dependencies, WDYT?

I'm not sure what the issue, i assume sendgrid (or one of it's depended packages) uses werkzeug without stating it as a dependency. When we had werkzeug as part of core we didn't notice it because it was installed any way.
I guess we need to open a bug report to sendgrid about it.
It's ok that we added werkzeug as mitigation but we need to have a todo near it explaining to remove it when it's fixed (with link to the open issue to track)

@potiuk
Copy link
Copy Markdown
Member

potiuk commented Apr 10, 2025

Also werkzeug is now back as dependency of airflow, so we can remove it

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.

4 participants