Skip to content

Conversation

@yannlambret
Copy link
Contributor

Follow-up to #50446 and #50521:

  • Clean up requirements-flattening logic.
  • Add tests for requirements passed as multi-line strings.

Following @shahar1 PR (#50521), I cleaned up a little bit the flattening logic of
_iter_serializable_context_keys, in connection with this part of the __init__ function
of PythonVirtualenvOperator class:

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

As self.requirements is always a list starting from there, it simplifies the way we can process it later. I also add the use cases detailed in #50521 to the relevant test. Sorry for my initial over-simplification 🙏🏻

@amoghrajesh
Copy link
Contributor

No problem! Thanks for taking this on.

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

LGTM

@yannlambret yannlambret force-pushed the improve-testing-for-context-serialization branch 2 times, most recently from f2d7bc9 to f07de43 Compare May 14, 2025 15:46
@amoghrajesh
Copy link
Contributor

Test failures are unrelated:

  providers/atlassian/jira/tests/unit/atlassian/jira/operators/test_jira.py s [ 10%]
  sss                                                                      [ 10%]
  providers/atlassian/jira/tests/unit/atlassian/jira/sensors/test_jira.py s [ 10%]
                                                                           [ 10%]
  providers/celery/tests/unit/celery/cli/test_celery_command.py .          [ 10%]
  providers/celery/tests/unit/celery/executors/test_celery_executor.py ... [ 10%]
  .sss.
  Test failed with 143. Dumping logs

rebasing it

@amoghrajesh amoghrajesh requested a review from eladkal May 15, 2025 06:02
Follow-up to apache#50446 and apache#50521:

- Clean up requirements-flattening logic.
- Add tests for requirements passed as multi-line strings.
@yannlambret yannlambret force-pushed the improve-testing-for-context-serialization branch from aeafb68 to 8f6214d Compare May 16, 2025 06:12
@yannlambret
Copy link
Contributor Author

Hi @amoghrajesh, I've rebased my branch cleanly onto main (dropped the merge commit and cherry-picked my changes). Hope the tests pass 🤞🏻

@yannlambret
Copy link
Contributor Author

All good!

@yannlambret
Copy link
Contributor Author

Hello @eladkal, could you please review this?

@shahar1 shahar1 merged commit 4cf50a6 into apache:main May 18, 2025
95 checks passed
@yannlambret yannlambret deleted the improve-testing-for-context-serialization branch May 19, 2025 14:50
dadonnelly316 pushed a commit to dadonnelly316/airflow that referenced this pull request May 26, 2025
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
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants