Skip to content

Flattening the requirements input for python venv operators#50521

Merged
eladkal merged 2 commits intoapache:mainfrom
amoghrajesh:fixing-provider-tests
May 13, 2025
Merged

Flattening the requirements input for python venv operators#50521
eladkal merged 2 commits intoapache:mainfrom
amoghrajesh:fixing-provider-tests

Conversation

@amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented May 13, 2025

#50446 did some rework on the way requirements are parsed. This was leading to some of the traditional ways of passing requirements to the venv operators to break.

Tested with:

'funcsigs==0.4\nattrs==23.1.0'

['funcsigs==0.4', 'attrs==23.1.0']

['funcsigs==0.4\nattrs==23.1.0']

'pendulum==2.1.2  # pinned version\nattrs==23.1.0  # optional'

['   pendulum==2.1.2   ', '# comment only', 'attrs==23.1.0   # inline comment']

['apache-airflow==2.8.0', 'something==1.0']

All good now.

Example failures: https://github.com/apache/airflow/actions/runs/14986598214

FAILED providers/standard/tests/unit/standard/decorators/test_python_virtualenv.py::TestPythonVirtualenvDecorator::test_with_requirements_file[pickle] - ValueError: Invalid requirement 'funcsigs==0.4
attrs==23.1.0': Expected end or semicolon (after version specifier)
    funcsigs==0.4
attrs==23.1.0
            ~~~~~^
FAILED providers/standard/tests/unit/standard/decorators/test_python_virtualenv.py::TestPythonVirtualenvDecorator::test_with_requirements_file[dill] - ValueError: Invalid requirement 'funcsigs==0.4
attrs==23.1.0': Expected end or semicolon (after version specifier)
    funcsigs==0.4
attrs==23.1.0
            ~~~~~^
FAILED providers/standard/tests/unit/standard/decorators/test_python_virtualenv.py::TestPythonVirtualenvDecorator::test_with_requirements_file[cloudpickle] - ValueError: Invalid requirement 'funcsigs==0.4
attrs==23.1.0': Expected end or semicolon (after version specifier)
    funcsigs==0.4
attrs==23.1.0
            ~~~~~^
FAILED providers/standard/tests/unit/standard/decorators/test_python_virtualenv.py::TestPythonVirtualenvDecorator::test_with_requirements_file[default] - ValueError: Invalid requirement 'funcsigs==0.4
attrs==23.1.0': Expected end or semicolon (after version specifier)
    funcsigs==0.4
attrs==23.1.0

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

@amoghrajesh amoghrajesh changed the title Flattening the requirements for python virtual env operator Flattening the requirements input for python venv operators May 13, 2025
@guan404ming guan404ming mentioned this pull request May 13, 2025
Merged
@amoghrajesh amoghrajesh self-assigned this May 13, 2025
@amoghrajesh amoghrajesh mentioned this pull request May 13, 2025
Merged
@eladkal
Copy link
Contributor

eladkal commented May 13, 2025

tests failure not related. merging

@eladkal eladkal merged commit 8df6a56 into apache:main May 13, 2025
93 of 94 checks passed
@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 13, 2025

Teat failures already fixed on main

@potiuk
Copy link
Member

potiuk commented May 13, 2025

Nice!

yannlambret
yannlambret reviewed May 13, 2025
View reviewed changes
providers/standard/src/airflow/providers/standard/operators/python.py
else:
for raw_str in self.requirements:
requirements_iterable = []
if isinstance(self.requirements, str):
Copy link
Contributor

@yannlambret yannlambret May 13, 2025

Choose a reason for hiding this comment

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

Sorry, I missed the fact that requirements could be other thing than a list. But based on the constructor code, it looks impossible to get a string here:

        elif isinstance(requirements, str):
            self.requirements = [requirements]

Copy link
Contributor Author

@amoghrajesh amoghrajesh May 14, 2025

Choose a reason for hiding this comment

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

Good call.

yannlambret added a commit to yannlambret/airflow that referenced this pull request May 13, 2025
@yannlambret
Improve testing for context serialization
d2dcb82 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
@yannlambret yannlambret mentioned this pull request May 13, 2025
Merged
yannlambret added a commit to yannlambret/airflow that referenced this pull request May 14, 2025
@yannlambret
Improve testing for context serialization
f2d7bc9 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
@eladkal eladkal mentioned this pull request May 14, 2025
Closed
yannlambret added a commit to yannlambret/airflow that referenced this pull request May 14, 2025
@yannlambret
Improve testing for context serialization
f07de43 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
yannlambret added a commit to yannlambret/airflow that referenced this pull request May 16, 2025
@yannlambret
Improve testing for context serialization
8f6214d 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
shahar1 pushed a commit that referenced this pull request May 18, 2025
@yannlambret
Improve testing for context serialization (#50566)
4cf50a6 
Follow-up to #50446 and #50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
dadonnelly316 pushed a commit to dadonnelly316/airflow that referenced this pull request May 26, 2025
@yannlambret @dadonnelly316
Improve testing for context serialization (apache#50566)
0f04f30 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
@amoghrajesh @sanederchik
Flattening the requirements input for python venv operators (apache…
669fd52 
…#50521)

* Flattening the requirements for python virtual env operator

* Refine BigQuery test with field-wise comparison of access_entries
sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
@yannlambret @sanederchik
Improve testing for context serialization (apache#50566)
afd6b8b 
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eladkal eladkal eladkal approved these changes

@potiuk potiuk Awaiting requested review from potiuk

@kaxil kaxil Awaiting requested review from kaxil

@gopidesupavan gopidesupavan Awaiting requested review from gopidesupavan

+1 more reviewer

@yannlambret yannlambret yannlambret left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@amoghrajesh amoghrajesh

Labels

area:providers provider:standard

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amoghrajesh @eladkal @potiuk @yannlambret