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 2022/10/27 15:45:50 UTC
[GitHub] [airflow] jtommi opened a new issue, #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
jtommi opened a new issue, #27328:
URL: https://github.com/apache/airflow/issues/27328
### Apache Airflow Provider(s)
sftp
### Versions of Apache Airflow Providers
apache-airflow-providers-sftp==4.1.0
### Apache Airflow version
2.4.2 Python 3.10
### Operating System
Debian 11 (Official docker image)
### Deployment
Docker-Compose
### Deployment details
Base image is apache/airflow:2.4.2-python3.10
### What happened
When combining Taskflow API and SFTPOperator, it throws an exception that didn't happen with apache-airflow-providers-sftp 4.0.0
### What you think should happen instead
The DAG should work as expected
### How to reproduce
```python
import pendulum
from airflow import DAG
from airflow.decorators import task
from airflow.providers.sftp.operators.sftp import SFTPOperator
with DAG(
"example_sftp",
schedule="@once",
start_date=pendulum.datetime(2021, 1, 1, tz="UTC"),
catchup=False,
tags=["example"],
) as dag:
@task
def get_file_path():
return "test.csv"
local_filepath = get_file_path()
upload = SFTPOperator(
task_id=f"upload_file_to_sftp",
ssh_conn_id="sftp_connection",
local_filepath=local_filepath,
remote_filepath="test.csv",
)
```
### Anything else
```logs
[2022-10-27T15:21:38.106+0000]` {logging_mixin.py:120} INFO - [2022-10-27T15:21:38.102+0000] {dagbag.py:342} ERROR - Failed to import: /opt/airflow/dags/test.py
Traceback (most recent call last):
File "/home/airflow/.local/lib/python3.10/site-packages/airflow/models/dagbag.py", line 338, in parse
loader.exec_module(new_module)
File "<frozen importlib._bootstrap_external>", line 883, in exec_module
File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
File "/opt/airflow/dags/test.py", line 21, in <module>
upload = SFTPOperator(
File "/home/airflow/.local/lib/python3.10/site-packages/airflow/models/baseoperator.py", line 408, in apply_defaults
result = func(self, **kwargs, default_args=default_args)
File "/home/airflow/.local/lib/python3.10/site-packages/airflow/providers/sftp/operators/sftp.py", line 116, in __init__
if len(self.local_filepath) != len(self.remote_filepath):
TypeError: object of type 'PlainXComArg' has no len()
```
It looks like the offending code was introduced in commit 5f073e38dd46217b64dbc16d7b1055d89e8c3459
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1386626513
Personally I think the eventual goal would be to ban any logic in `__init__` for custom operators since all logic should be in `execute` anyway, and attribute assignment can be trivially covered by `@dataclasses.dataclass` or `@attr.define`. Would that be a viable goal? Maybe we could invest in changing most operators to use one of those two, and write a pre-commit hook to ban implementing `__init__` in custom operators (with limited exceptions of course).
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1398832969
> I like this. Though it will take quite some tmie I am afraid - unless we can do most of it semi-automatically (which I think might be quite possible).
Created https://github.com/apache/airflow/issues/29069
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1337215045
Yeah. It's been released in 2.5.0 so just upgrading to 2.5.0 should be enough to test 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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1384367659
Yep. It looks like this change https://github.com/apache/airflow/pull/26666 broke it (the number indicates it's a bit hell-ish change). the checks are in constructor but should be in the execute() method.
--
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 closed issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
URL: https://github.com/apache/airflow/issues/27328
--
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] jtommi commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
jtommi commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1383856991
@potiuk I finally upgraded to 2.5.0 (-python3.10) and the issue persists.
The version of apache-airflow-providers-sftp in that image is 4.2.0
I also tried apache-airflow-providers-sftp 4.2.1, but still same issue.
Pinning apache-airflow-providers-sftp to 4.0.0 still fixes the issue
--
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] jtommi commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
jtommi commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1337020244
> This should also be fixed regardless of the SFTP provider version by #27251
Great, I'll subscribe to #27251 and verify that it fixes the issue.
Thanks @potiuk for letting me know
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1386767606
> Personally I think the eventual goal would be to ban any logic in `__init__` for custom operators since all logic should be in `execute` anyway, and attribute assignment can be trivially covered by `@dataclasses.dataclass` or `@attr.define`. Would that be a viable goal? Maybe we could invest in changing most operators to use one of those two, and write a pre-commit hook to ban implementing `__init__` in custom operators (with limited exceptions of course).
I like this. Though it will take quite some tmie I am afraid - unless we can do most of it semi-automatically (which I think might be quite possible).
--
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 closed issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
URL: https://github.com/apache/airflow/issues/27328
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1336490676
This should also be fixed regardless of the SFTP provider version by https://github.com/apache/airflow/pull/27251
--
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] jtommi commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
jtommi commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1296853078
The thing is: either I broke (or didn't find) the code that made Taskflow work in version 4.0 of this package, or my test is incorrect.
--
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] jtommi commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
jtommi commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1294988085
@uranusjr Thanks for the initial direction to fixing this.
I drafted #27346.
I think the initial bug is fixed, but my test that checks for compatibility is failing.
https://github.com/apache/airflow/blob/c84831151a1e2be018f4280180e3002bcf3153f4/tests/providers/sftp/operators/test_sftp.py#L413
The reason is `expected str, bytes or os.PathLike object, not PlainXComArg` on https://github.com/apache/airflow/blob/c84831151a1e2be018f4280180e3002bcf3153f4/airflow/providers/sftp/operators/sftp.py#L173
The thing is that I don't understand why the code worked with version 2 of the provider package (I actually didn't check with version 4, but I didn't get import errors).
The only change from previous versions of the code is that before it did `os.path.dirname(self.local_filepath)` and now `os.path.dirname(local_filepath)`, but I couldn't find any code that converted a PlainXComArg into a string.
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1384371084
Woul you like to fix it @jtommi ?
And @uranusjr - we both approved the change. I am starting to think that we should attempt to do do some automated prevention of similar issues - this is all too easy (and seemingly obvious) to perform such valiations and conversions in the constructor rather than in execute methods. And apparently it is easy to get past the aproval of both of us, so possibly that's a sign we should summon our CI to prevent such things. Though I am not sure yet how to do 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] uranusjr commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1295938513
All the checks should be moved to `execute` anyway, just do that and don’t worry that much about the previous logic.
--
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] o-nikolas commented on issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1294411017
Thanks for the bug report @jtommi!
I see that you've checked that you're willing to submit a PR, so I have assigned the task to you :smiley:
--
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 issue #27328: SFTPOperator throws object of type 'PlainXComArg' has no len() when using with Taskflow API
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #27328:
URL: https://github.com/apache/airflow/issues/27328#issuecomment-1294500097
Value checks in operators should be done in `execute`, not `__init__`.
--
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