Skip to content

Conversation

ashb
Copy link
Member

@ashb ashb commented Mar 20, 2025

Some of these deps, like python-nvd3 or markdown-it-py, are simply not used
anymore, while others are still used and the dep has been moved to the fab
provider where it is actually used.


^ 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.

@ashb ashb added the full tests needed We need to run full set of tests for this PR to merge label Mar 20, 2025
@ashb ashb closed this Mar 20, 2025
@ashb ashb reopened this Mar 20, 2025
@vincbeck
Copy link
Contributor

Love to see all these deps moving away from airflow-core!

Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

Overall LGTM, let's see if tests are passing

@vincbeck
Copy link
Contributor

Tests are failing because of JSONProvider. But it does not seem to be used, so I guess we can just remove it

@ashb ashb requested review from eladkal and o-nikolas as code owners March 20, 2025 14:41
@jedcunningham
Copy link
Member

Yours is a bit more comprehensive than mine (#47958). I can do the password extra removal separately.

@potiuk
Copy link
Member

potiuk commented Mar 21, 2025

Better to re-do it in the new form after #47798

@potiuk
Copy link
Member

potiuk commented Mar 22, 2025

Should be rebased now after airflow-core move as the dependncies moved to "airflow-core/pyproject.toml"

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Needs rebase / conflicts removal but it's very cool change :)

@ashb ashb force-pushed the remove-old-www-deps branch 2 times, most recently from adcff87 to e8149ac Compare March 23, 2025 11:30
@ashb
Copy link
Member Author

ashb commented Mar 23, 2025

Deleted hatch_build by mistake. Ooops :)

@ashb ashb force-pushed the remove-old-www-deps branch from e8149ac to d60eb1f Compare March 23, 2025 11:44
@potiuk
Copy link
Member

potiuk commented Mar 23, 2025

Deleted hatch_build by mistake. Ooops :)

FYI With getting rid of pip I am eyeying hatch_build.py removal anyway ;) (for the meta package at least - custom build hooks for asset compilation and git hook will remain in airflow-core - but neither with root pyproject.toml nor in apache-airflow we will have dynamic dependencies, I believe.

@ashb ashb force-pushed the remove-old-www-deps branch from d60eb1f to 01b5075 Compare March 23, 2025 13:00
@ashb
Copy link
Member Author

ashb commented Mar 23, 2025

Hmmm, somewhere we are still/yet again missing the dep on werkzeug:

 __ ERROR collecting airflow-core/tests/unit/always/test_providers_manager.py ___
airflow-core/tests/unit/always/test_providers_manager.py:28: in <module>
    from flask_babel import lazy_gettext
/usr/local/lib/python3.10/site-packages/flask_babel/__init__.py:15: in <module>
    from flask import current_app, request
/usr/local/lib/python3.10/site-packages/flask/__init__.py:4: in <module>
    from . import json as json
/usr/local/lib/python3.10/site-packages/flask/json/__init__.py:8: in <module>
    from ..globals import current_app
/usr/local/lib/python3.10/site-packages/flask/globals.py:56: in <module>
    app_ctx: "AppContext" = LocalProxy(  # type: ignore[assignment]
E   TypeError: LocalProxy.__init__() got an unexpected keyword argument 'unbound_message'

I've run out of spare time this weekend, I'll take a look on Monday

@potiuk
Copy link
Member

potiuk commented Mar 23, 2025

__ ERROR collecting airflow-core/tests/unit/always/test_providers_manager.py ___

Looks tests only. Possibly we shoud move the tests that use it to FAB (they are really testing "fab provider" variant of the providers_manager)

@potiuk
Copy link
Member

potiuk commented Mar 23, 2025

Fix here: #48113

@ashb ashb force-pushed the remove-old-www-deps branch from 331d651 to 7efd12d Compare March 24, 2025 09:57
@potiuk
Copy link
Member

potiuk commented Mar 24, 2025

seems like fast_api/svcs has another implicit flask dependency

@potiuk
Copy link
Member

potiuk commented Mar 24, 2025

seems like fast_api/svcs has another implicit flask dependency

but likely it's because it is there in an old version of it - as only "lowest dependencies" are impacted.

@potiuk
Copy link
Member

potiuk commented Mar 24, 2025

Most likely:

   - fastapi==0.115.12
   + fastapi==0.112.4

@potiuk
Copy link
Member

potiuk commented Mar 24, 2025

Possibly fastapi/fastapi#12747 (0.115.5)

@ashb
Copy link
Member Author

ashb commented Mar 24, 2025

Looking

@ashb
Copy link
Member Author

ashb commented Mar 24, 2025

This is an odd one:

E   sqlalchemy.exc.ArgumentError: Type annotation for "TaskInstance.dag_model" can't be correctly interpreted for Annotated Declarative Table form.  ORM annotations should normally make use of the ``Mapped[]`` generic type, or other ORM-compatible generic type, as a container for the actual type, which indicates the intent that the attribute is mapped. Class variables that are not intended to be mapped by the ORM should use ClassVar[].  To allow Annotated Declarative to disregard legacy annotations which don't use Mapped[] to pass, set "__allow_unmapped__ = True" on the class or a superclass this class. (Background on this error at: https://sqlalche.me/e/20/zlpr)
=========================== short test summary info ============================
ERROR kubernetes_tests/test_kubernetes_pod_operator.py - sqlalchemy.exc.ArgumentError: Type annotation for "TaskInstance.dag_model" can't be correctly interpreted for Annotated Declarative Table form.  ORM annotations should normally make use of the ``Mapped[]`` generic type, or other ORM-compatible generic type, as a container for the actual type, which indicates the intent that the attribute is mapped. Class variables that are not intended to be mapped by the ORM should use ClassVar[].  To allow Annotated Declarative to disregard legacy annotations which don't use Mapped[] to pass, set "__allow_unmapped__ = True" on the class or a superclass this class. (Background on this error at: https://sqlalche.me/e/20/zlpr)

@ashb ashb force-pushed the remove-old-www-deps branch from 7efd12d to 215a3e9 Compare March 24, 2025 12:43
@ashb
Copy link
Member Author

ashb commented Mar 24, 2025

Ah, I didn't resolve conflicts correctly. Lets try this

ashb added 2 commits March 24, 2025 13:44
Some of these deps, like python-nvd3 or markdown-it-py, are simply not used
anymore, while others are still used and the dep has been moved to the fab
provider where it is actually used.
@ashb ashb force-pushed the remove-old-www-deps branch from 215a3e9 to 3148d60 Compare March 24, 2025 13:44
@ashb ashb merged commit d1b80e4 into main Mar 24, 2025
148 checks passed
@ashb
Copy link
Member Author

ashb commented Mar 24, 2025

Thanks for the assist @potiuk

@ashb ashb deleted the remove-old-www-deps branch March 25, 2025 10:07
pankajkoti pushed a commit to astronomer/airflow that referenced this pull request Mar 28, 2025
Some of these deps, like python-nvd3 or markdown-it-py, are simply not used
anymore, while others are still used and the dep has been moved to the fab
provider where it is actually used.
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Apr 2, 2025
Some of these deps, like python-nvd3 or markdown-it-py, are simply not used
anymore, while others are still used and the dep has been moved to the fab
provider where it is actually used.
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
Some of these deps, like python-nvd3 or markdown-it-py, are simply not used
anymore, while others are still used and the dep has been moved to the fab
provider where it is actually used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers full tests needed We need to run full set of tests for this PR to merge provider:fab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants