-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
Restrict marshmallow==3.23 #45442
Restrict marshmallow==3.23 #45442
Conversation
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.
Thanks!
The failure seems to be due to flakiness, retriggered. |
Nice! |
@gopidesupavan thank you for finding and fixing this. The effect of marshmallow's breaking change was significant for me Since this does schema validation, this effectively caused all of my Airflow API requests to suddenly start 400'ing on my next deploy (deploys re pip-install things when the container is rebuilt). I have business critical functions that depend on programatically launching DAGs. All of those broke suddenly. |
marshmallow 3.24 doesn't actually have a breaking change: it only raises warnings for usages that are changed or removed in 4.0. The issue is that marshmallow-sqlalchemy (a dependency of Flask-AppBuilder) needs to be upgraded to version 1.1.1 to work correctly with marshmallow. So I think this should be fixed by relaxing the version constraint here: https://github.com/dpgaspar/Flask-AppBuilder/blob/418ab8a93907669be4ccbb99d7aefa5283f3e013/setup.py#L61 Once I get my Flask-AppBuilder dev environment set up, I can send a PR with that change |
Nice find . Thanks for following it up! |
i just released marshmallow 3.24.2, which fixes compatibility with older marshmallow-sqlalchemy. so we no longer need to wait for Flask-AppBuilder. #45499 reverts this as it's no longer be necessary to pin marshmallow |
Cool. I rebased the PR and added legacy api label - let's see if all tests pass :) |
Co-authored-by: Amogh Desai <[email protected]>
Co-authored-by: Amogh Desai <[email protected]>
Co-authored-by: Amogh Desai <[email protected]>
It looks like marshmallow==3.24 introduced the breaking changes to
https://github.com/marshmallow-code/marshmallow/blob/dev/CHANGELOG.rst#3240-2025-01-06
Field <marshmallow.fields.Field>, Mapping <marshmallow.fields.Mapping>, and Number <marshmallow.fields.Number> should no longer be used as fields within schemas. Use their subclasses instead.
Likely this PR marshmallow-code/marshmallow#2723
We use <marshmallow.fields.Number> in https://github.com/apache/airflow/blob/main/airflow/api_connexion/schemas/task_schema.py#L51. Likely these updates required?
CI is failing https://github.com/apache/airflow/actions/runs/12639019672/job/35217178280#step:6:4004
^ 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 newsfragments.