You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/08/29 17:49:27 UTC
[GitHub] [airflow] rounakdatta opened a new pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
rounakdatta opened a new pull request #17349:
URL: https://github.com/apache/airflow/pull/17349
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Closes: https://github.com/apache/airflow/issues/16037
This pull request addresses the changes discussed in the aforementioned issue.
- [x] Added new unit tests
- [x] This change is not breaking
- [x] Updating documentation of the operator
Changes:
- Support for passing a `requirements.txt` file in the `requirements` field of the PythonVirtualenvOperator.
- The file can be named `*.txt` (anything ending with .txt), and can be jinja templated.
Note: `template_searchpath` must be appropriately set while using arbitrary locations of the template file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr merged pull request #17349: #16037 Templated requirements.txt in Python operators
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #17349:
URL: https://github.com/apache/airflow/pull/17349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-961879110
@rounakdatta can you please take a look at the error?
```
ERROR [airflow.models.dagbag.DagBag] Failed to import: /opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py
Traceback (most recent call last):
File "/opt/airflow/airflow/models/dagbag.py", line 331, in _load_modules_from_file
loader.exec_module(new_module)
File "<frozen importlib._bootstrap_external>", line 850, in exec_module
File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
File "/opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py", line 22, in <module>
from airflow.decorators import dag, task
File "/opt/airflow/airflow/decorators/__init__.py", line 20, in <module>
from airflow.decorators.python import PythonDecoratorMixin, python_task # noqa
File "/opt/airflow/airflow/decorators/python.py", line 21, in <module>
from airflow.operators.python import PythonOperator
File "/opt/airflow/airflow/operators/python.py", line 252, in <module>
class PythonVirtualenvOperator(PythonOperator):
File "/opt/airflow/airflow/operators/python.py", line 341, in PythonVirtualenvOperator
requirements: Optional[Iterable[str], str] = None,
File "/usr/local/lib/python3.9/typing.py", line 275, in inner
return func(*args, **kwds)
File "/usr/local/lib/python3.9/typing.py", line 352, in __getitem__
return self._getitem(self, parameters)
File "/usr/local/lib/python3.9/typing.py", line 475, in Optional
arg = _type_check(parameters, f"{self} requires a single type.")
File "/usr/local/lib/python3.9/typing.py", line 164, in _type_check
raise TypeError(f"{msg} Got {arg!r:.100}.")
TypeError: typing.Optional requires a single type. Got (typing.Iterable[str], <class 'str'>).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r680879448
##########
File path: airflow/operators/python.py
##########
@@ -333,20 +337,31 @@ def __init__(
templates_exts=templates_exts,
**kwargs,
)
- self.requirements = list(requirements or [])
+ if not isinstance(requirements, str):
+ self.requirements = list(requirements or [])
+ else:
+ self.requirements = requirements
self.string_args = string_args or []
self.python_version = python_version
self.use_dill = use_dill
self.system_site_packages = system_site_packages
- if not self.system_site_packages and self.use_dill and 'dill' not in self.requirements:
- self.requirements.append('dill')
self.pickling_library = dill if self.use_dill else pickle
+ def pre_execute(self, context: Any):
+ if isinstance(self.requirements, list):
+ return
+
+ import pkg_resources
+ self.requirements = [str(req) for req in pkg_resources.parse_requirements(self.requirements)]
Review comment:
Ack, makes sense, let me work on that change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r681669098
##########
File path: airflow/operators/python.py
##########
@@ -333,20 +337,31 @@ def __init__(
templates_exts=templates_exts,
**kwargs,
)
- self.requirements = list(requirements or [])
+ if not isinstance(requirements, str):
+ self.requirements = list(requirements or [])
+ else:
+ self.requirements = requirements
self.string_args = string_args or []
self.python_version = python_version
self.use_dill = use_dill
self.system_site_packages = system_site_packages
- if not self.system_site_packages and self.use_dill and 'dill' not in self.requirements:
- self.requirements.append('dill')
self.pickling_library = dill if self.use_dill else pickle
+ def pre_execute(self, context: Any):
+ if isinstance(self.requirements, list):
+ return
+
+ import pkg_resources
+ self.requirements = [str(req) for req in pkg_resources.parse_requirements(self.requirements)]
Review comment:
@uranusjr I have tried an implementation of your proposed approach, can you once re-review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-980437988
There are failures around scheduling in `test_scheduler_job.py` and in `test_redis_publish.py`, do we have any actionables here? Possibly transient?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-910592541
Still some tests failing consistently @rounakdatta :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-989311500
Merge Conflicts :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-909321132
Phew, I've rebased it against the apache main. @potiuk can we run the CI tests once again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r765793081
##########
File path: airflow/operators/python.py
##########
@@ -405,6 +409,19 @@ def execute(self, context: Context):
def execute_callable(self):
with TemporaryDirectory(prefix='venv') as tmp_dir:
+ requirements_file_name = f'{tmp_dir}/requirements.txt'
+
+ if isinstance(self.requirements, List):
Review comment:
```suggestion
if isinstance(self.requirements, list):
```
doesn't make a difference but just for consistency across code-base. No strong preference
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r751145362
##########
File path: airflow/operators/python.py
##########
@@ -334,7 +338,7 @@ def __init__(
self,
*,
python_callable: Callable,
- requirements: Optional[Iterable[str]] = None,
+ requirements: Optional[Iterable[str], str] = None,
Review comment:
```suggestion
requirements: Union[None, Iterable[str], str] = None,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-980307970
@eladkal sorry for the delay, I've just rebased, can you trigger the tests?
(I wasn't able to fully reproduce the previous provider class failing tests, but from the last attempt, those seem to have passed ๐)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r739611995
##########
File path: airflow/operators/python.py
##########
@@ -300,7 +304,7 @@ def __init__(
self,
*,
python_callable: Callable,
- requirements: Optional[Iterable[str]] = None,
+ requirements: Union[Iterable[str], str, None] = None,
Review comment:
Makes sense, addressed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908481746
> One check is failing where `apache-airflow[devel-ci]` package is failing to install because of a dependency version incompatibility. I believe this didn't get introduced due to my change ๐ฌ
I think you need to rebase - that was celery-5 change that got merged recently, and it is conflcting unless you rebase to latest `main`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908211891
Looks like on the tests is stuck, shall I push an empty commit @uranusjr ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-955152630
@eladkal @turbaszek done rebasing :)
Can you help trigger the tests once more?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #17349: #16037 Templated requirements.txt in Python operators
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-1007450682
Awesome work, congrats on your first merged pull request!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr merged pull request #17349: #16037 Templated requirements.txt in Python operators
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #17349:
URL: https://github.com/apache/airflow/pull/17349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr closed pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #17349:
URL: https://github.com/apache/airflow/pull/17349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908215066
No need, I just re-triggered them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr closed pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #17349:
URL: https://github.com/apache/airflow/pull/17349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r680677265
##########
File path: airflow/operators/python.py
##########
@@ -333,20 +337,31 @@ def __init__(
templates_exts=templates_exts,
**kwargs,
)
- self.requirements = list(requirements or [])
+ if not isinstance(requirements, str):
+ self.requirements = list(requirements or [])
+ else:
+ self.requirements = requirements
self.string_args = string_args or []
self.python_version = python_version
self.use_dill = use_dill
self.system_site_packages = system_site_packages
- if not self.system_site_packages and self.use_dill and 'dill' not in self.requirements:
- self.requirements.append('dill')
self.pickling_library = dill if self.use_dill else pickle
+ def pre_execute(self, context: Any):
+ if isinstance(self.requirements, list):
+ return
+
+ import pkg_resources
+ self.requirements = [str(req) for req in pkg_resources.parse_requirements(self.requirements)]
Review comment:
The requirements file format can do more than listing requirements, and those cannot be parsed by `parse_requirements` (I donโt remember whether setuptools would throw or just silently drop them.) So instead of trying to pre-parse the content, I would try to parse it to pip instead.
* Remove `pre_execute`
* Change `prepare_virtualenv` to accept `requirements` as str (plus the currently supported list form)
* When doing `pip install`, write the str type `requirements` to a temporary file for `pip intall -r`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r739462373
##########
File path: airflow/operators/python.py
##########
@@ -300,7 +304,7 @@ def __init__(
self,
*,
python_callable: Callable,
- requirements: Optional[Iterable[str]] = None,
+ requirements: Union[Iterable[str], str, None] = None,
Review comment:
I think we usually tend to use `Optional` instead of `Union[..., None]` additionaly `Optional` marks the argument as... optional ๐
```suggestion
requirements: Optional[Union[Iterable[str], str]] = None,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-961879110
@rounakdatta can you please take a look at the error?
```
ERROR [airflow.models.dagbag.DagBag] Failed to import: /opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py
Traceback (most recent call last):
File "/opt/airflow/airflow/models/dagbag.py", line 331, in _load_modules_from_file
loader.exec_module(new_module)
File "<frozen importlib._bootstrap_external>", line 850, in exec_module
File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
File "/opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py", line 22, in <module>
from airflow.decorators import dag, task
File "/opt/airflow/airflow/decorators/__init__.py", line 20, in <module>
from airflow.decorators.python import PythonDecoratorMixin, python_task # noqa
File "/opt/airflow/airflow/decorators/python.py", line 21, in <module>
from airflow.operators.python import PythonOperator
File "/opt/airflow/airflow/operators/python.py", line 252, in <module>
class PythonVirtualenvOperator(PythonOperator):
File "/opt/airflow/airflow/operators/python.py", line 341, in PythonVirtualenvOperator
requirements: Optional[Iterable[str], str] = None,
File "/usr/local/lib/python3.9/typing.py", line 275, in inner
return func(*args, **kwds)
File "/usr/local/lib/python3.9/typing.py", line 352, in __getitem__
return self._getitem(self, parameters)
File "/usr/local/lib/python3.9/typing.py", line 475, in Optional
arg = _type_check(parameters, f"{self} requires a single type.")
File "/usr/local/lib/python3.9/typing.py", line 164, in _type_check
raise TypeError(f"{msg} Got {arg!r:.100}.")
TypeError: typing.Optional requires a single type. Got (typing.Iterable[str], <class 'str'>).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #17349: #16037 Templated requirements.txt in Python operators
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-1007450682
Awesome work, congrats on your first merged pull request!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-936797911
@rounakdatta can you please rebase?
The test failure
`tests/www/views/test_views_dagrun.py::test_delete_dagrun: AssertionError: assert 1 == 0`
seems to be related to https://github.com/apache/airflow/pull/16634 I think rebase should solve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-910447473
And now tests are failing (and look like related to your changes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta edited a comment on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta edited a comment on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908211891
Looks like one of the tests is stuck, shall I push an empty commit @uranusjr ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908201155
Come on CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r698029991
##########
File path: airflow/utils/python_virtualenv.py
##########
@@ -87,14 +87,15 @@ def prepare_virtualenv(
:param system_site_packages: Whether to include system_site_packages in your virtualenv.
See virtualenv documentation for more information.
:type system_site_packages: bool
- :param requirements: List of additional python packages
- :type requirements: List[str]
+ :param requirements: Path to the requirements.txt file
Review comment:
Addressed! ๐
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r697905705
##########
File path: airflow/utils/python_virtualenv.py
##########
@@ -87,14 +87,15 @@ def prepare_virtualenv(
:param system_site_packages: Whether to include system_site_packages in your virtualenv.
See virtualenv documentation for more information.
:type system_site_packages: bool
- :param requirements: List of additional python packages
- :type requirements: List[str]
+ :param requirements: Path to the requirements.txt file
Review comment:
I think we should still (backwards compatibility) handle the case where requirements are List[str]. It will make it a bit more complex, but I think it is needed.
I propose to keep the old `requrements` handling only List[str] and add a new parameter `requirements_file_path` - and check if only one of those is passed (and act accordingly). I think it is very ambiguous to name `requirements` something that is path to requirements file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908481746
> One check is failing where `apache-airflow[devel-ci]` package is failing to install because of a dependency version incompatibility. I believe this didn't get introduced due to my change ๐ฌ
I think you need to rebase - that was celery-5 change that got merged recently, and it is conflcting unless you rebase to latest `main`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r765794620
##########
File path: airflow/utils/python_virtualenv.py
##########
@@ -89,12 +95,23 @@ def prepare_virtualenv(
:type system_site_packages: bool
:param requirements: List of additional python packages
:type requirements: List[str]
+ :param requirements_file_path: Path to the requirements.txt file
Review comment:
```suggestion
:param requirements_file_path: Path to the ``requirements.txt`` file
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-890106651
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-909321132
Phew, I've rebased it against the apache main. @potiuk can we run the CI tests once again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] rounakdatta commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
rounakdatta commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-908251840
One check is failing where `apache-airflow[devel-ci]` package is failing to install because of a dependency version incompatibility. I believe this didn't get introduced due to my change ๐ฌ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-989862126
tests are failing:
```
=================================== FAILURES ===================================
__________________ TestPythonVirtualenvOperator.test_add_dill __________________
self = <tests.operators.test_python.TestPythonVirtualenvOperator testMethod=test_add_dill>
def test_add_dill(self):
def f():
import dill # noqa: F401
import lazy_object_proxy # noqa: F401
> self._run_as_operator(f, use_dill=True, system_site_packages=False)
tests/operators/test_python.py:750:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/operators/test_python.py:742: in _run_as_operator
task.run(start_date=DEFAULT_DATE, end_date=DEFAULT_DATE)
airflow/utils/session.py:69: in wrapper
return func(*args, session=session, **kwargs)
airflow/models/baseoperator.py:1348: in run
session=session,
airflow/utils/session.py:66: in wrapper
return func(*args, **kwargs)
airflow/models/taskinstance.py:1636: in run
session=session,
airflow/utils/session.py:66: in wrapper
return func(*args, **kwargs)
airflow/models/taskinstance.py:1331: in _run_raw_task
self._execute_task_with_callbacks(context)
airflow/models/taskinstance.py:1457: in _execute_task_with_callbacks
result = self._execute_task(context, self.task)
airflow/models/taskinstance.py:1513: in _execute_task
result = execute_callable(context=context)
airflow/operators/python.py:408: in execute
return super().execute(context=serializable_context)
airflow/operators/python.py:181: in execute
return_value = self.execute_callable()
airflow/operators/python.py:460: in execute_callable
string_args_filename,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#discussion_r745185457
##########
File path: airflow/operators/python.py
##########
@@ -300,7 +304,7 @@ def __init__(
self,
*,
python_callable: Callable,
- requirements: Optional[Iterable[str]] = None,
+ requirements: Union[Iterable[str], str, None] = None,
Review comment:
FWIW the two are equivalent, `Optional[X]` is just syntax sugar to `Union[X, None]` (and IIRC Mypy normalises the former to latter internally, not sure)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-961879110
@rounakdatta can you please take a look at the error?
```
ERROR [airflow.models.dagbag.DagBag] Failed to import: /opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py
Traceback (most recent call last):
File "/opt/airflow/airflow/models/dagbag.py", line 331, in _load_modules_from_file
loader.exec_module(new_module)
File "<frozen importlib._bootstrap_external>", line 850, in exec_module
File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
File "/opt/airflow/airflow/example_dags/tutorial_taskflow_api_etl_virtualenv.py", line 22, in <module>
from airflow.decorators import dag, task
File "/opt/airflow/airflow/decorators/__init__.py", line 20, in <module>
from airflow.decorators.python import PythonDecoratorMixin, python_task # noqa
File "/opt/airflow/airflow/decorators/python.py", line 21, in <module>
from airflow.operators.python import PythonOperator
File "/opt/airflow/airflow/operators/python.py", line 252, in <module>
class PythonVirtualenvOperator(PythonOperator):
File "/opt/airflow/airflow/operators/python.py", line 341, in PythonVirtualenvOperator
requirements: Optional[Iterable[str], str] = None,
File "/usr/local/lib/python3.9/typing.py", line 275, in inner
return func(*args, **kwds)
File "/usr/local/lib/python3.9/typing.py", line 352, in __getitem__
return self._getitem(self, parameters)
File "/usr/local/lib/python3.9/typing.py", line 475, in Optional
arg = _type_check(parameters, f"{self} requires a single type.")
File "/usr/local/lib/python3.9/typing.py", line 164, in _type_check
raise TypeError(f"{msg} Got {arg!r:.100}.")
TypeError: typing.Optional requires a single type. Got (typing.Iterable[str], <class 'str'>).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-978903919
tests are still failing but I don't see something related to this PR. We had some trouble with the CI lately so probably another rebase should give us a green build.
@rounakdatta can you please do another rebase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #17349: #16037 Add support for passing templated requirements.txt in PythonVirtualenvOperator
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #17349:
URL: https://github.com/apache/airflow/pull/17349#issuecomment-1006307729
Those `test_add_dill` tests shouldnโt import `lazy_object_proxy`, that is not guaranteed to be injected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org