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/11/30 12:07:51 UTC
[GitHub] [airflow] potiuk opened a new issue #19891: Re-enable MyPy
potiuk opened a new issue #19891:
URL: https://github.com/apache/airflow/issues/19891
### Body
For a few weeks MyPy checks have been disabled after the switch to Python 3.7 (per #19317).
We should, however, re-enable it back as it is very useful in catching a number of mistakes.
I am re-adding the mypy pre-commit now - with mypy bumped to 0.910. This version detects far more number of errors and we should fix them all before we switch the CI check back.
The way I brought it back https://github.com/apache/airflow/pull/19890
* mypy will be running for incremental changes in pre-commit, same as before. This will enable incremental fixes of the code changed by committers who use pre-commits locally
* mypy on CI runs in non-failing mode. When main pre-commit check is run, mypy is disabled, but then it is run as separate step (which does not fail but will show the result of running mypy on all our code). This will enable us to track the progress of fixes
We should make a concerted effort now and incrementally fix all the mypy incompatibilities - ideally package/by/package to avoid huge code reviews.
### Committer
- [X] I acknowledge that I am a maintainer/committer of the Apache Airflow project.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006491859
> We are doing it now :D taking Amazon Provider for now -- so the following files:
>
> * [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
> * [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
> * [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200)
> * [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
> * [ ] ( 1) airflow/providers/amazon/aws/transfers (Me) -> [Fix MyPy issues in `airflow/providers/amazon/aws/transfers` #20708](https://github.com/apache/airflow/pull/20708)
@kaxil , heads up some work for /aws/hooks was already done here - https://github.com/apache/airflow/pull/20353 , PR is not merged yet.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009754363
@ashb @uranusjr @jedcunningham (others): that actually reminded me that we have not decided what to do with .pyi files. As mentioned in https://github.com/apache/airflow/issues/19891#issuecomment-1001531572
> **This is a very good point. Technically we should do it before we reease the files in our source package. I am not sure if we want to release them, but this is another question which i think will need to be answered separately - and I think we shoud have a spearate discussion on how and whether to release them:
> * we could not release them and only leave them in the repo
> * we could release them in sources or
> * we could release a separate typeshed package for those who want to use mypy https://github.com/python/typeshed
I think some of the important stub files (Context especially) we should directly include in "airflow" package. But some others not (especially if we decide to leave them - for now at least - to solve some of the "default args + examples" case, we should not include). Currently we do not include any of the stub files (and we do not have a typeshed package).
We do not have a big number of stub files yet - most of them are provider's "workaround". I am quite sure we should include context.pyi - not sure about db_types/functools.
```
./airflow/utils/context.pyi
./airflow/migrations/db_types.pyi
./airflow/compat/functools.pyi
./airflow/providers/amazon/aws/operators/eks.pyi
./airflow/providers/amazon/aws/operators/s3.pyi
./airflow/providers/amazon/aws/sensors/eks.pyi
./airflow/providers/alibaba/cloud/operators/oss.pyi
./airflow/providers/microsoft/azure/operators/wasb_delete_blob.pyi
./airflow/providers/microsoft/azure/operators/cosmos.pyi
./airflow/providers/microsoft/azure/transfers/azure_blob_to_gcs.pyi
./airflow/providers/microsoft/azure/transfers/local_to_wasb.pyi
./airflow/providers/microsoft/azure/sensors/wasb.pyi
./airflow/providers/microsoft/azure/sensors/cosmos.pyi
./airflow/providers/asana/operators/asana_tasks.pyi
./airflow/providers/apache/cassandra/sensors/record.pyi
./airflow/providers/apache/cassandra/sensors/table.pyi
```
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers (we only include .py files). They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add those .pyi files, but adding # typeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level of the exampels (outside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003958947
I concur with @uranusjr assesment. Even in PEP 585 it is written:
> Usefulness of this syntax before PEP 585 is limited as external tooling like Mypy does not recognize standard collections as generic. Moreover, certain features of typing like type aliases or casting require putting types outside of annotations, in runtime context. While these are relatively less common than type annotations, it's important to allow using the same type syntax in all contexts. This is why starting with Python 3.9, the following collections become generic using __class_getitem__() to parameterize contained types:
I think this will be a good exercise to make when we will prepare to drop Python 3.8 and for now using `typing` is best (or actually until MyPy will start reporting that as a warning).
Actually (side comment), I do not really like the `abc` in Collections :). This ship has sailed already. But until the original collection.* is there this adds even more confusion (should collections.Sequence also be used as generic as well as collections.abc.Sequence ?) . The old collections.* imports will be removed in 3.9. But tt did not happen. They were finally dropped in 3.10, I believe. And only then we will have somewhat sane collections imports :). so we have indeed years until this becomes even a warning.
--
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] mingshi-wang commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
mingshi-wang commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002328580
I fixed mypy errors in airflow/decorators and then realized it was a dup effort. I'd be happy to help if there is any work not taken by anyone yet.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015640132
Looking at azure/example_dags and amazon/example_dags.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003374241
We are on definitely downward path:
* down to 39 packages
* down to 273 errrors
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1004002778
I agree with option 1 and think it will be necessary to do refactoring when Python 3.8 is retired for airflow. And I am sure there will be a lot more additions to Python during these 2 years that may affect future decisions and void any current that community can made.
Thanks for your views, I guess it's good to have in records that it was considered at this phase anyway.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982946395
Just checked.
Parallel pre-commit is useless for `mypy` - seems that it will not speedup the tests anyway as mypy pretty much loads as many python code as it reaches out from the files, and with our code structure, any import to airflow will pretty much load everything always. So when we run MyPy in parallel, My 16 cores are fully busy and produce the same result in 2m6s as single busy core in 2m13s. Results are the same.
I will turn on serial mode :).
Serial:
```
[jarek:~/code/airflow] enable-mypy-as-non-failing-step(+3/-4)+ 8s ± pre-commit run mypy --all-files 2>&1 | sed -r "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[mGK]//g" | grep "error:" | sort | awk 'FS=" " { print $1 }' | uniq > /tmp/res-serial.out
[jarek:~/code/airflow] enable-mypy-as-non-failing-step(+3/-4)+ 2m13s ± wc /tmp/res-serial.out
963 963 48752 /tmp/res-serial.out
```
Paralel:
```
[jarek:~/code/airflow] enable-mypy-as-non-failing-step(+3/-4)+ ± pre-commit run mypy --all-files 2>&1 | sed -r "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[mGK]//g" | grep "error:" | sort | awk 'FS=" " { print $1 }' | uniq > /tmp/res-paralle.out
[jarek:~/code/airflow] enable-mypy-as-non-failing-step(+1/-2)+ 2m6s ± wc /tmp/res-paralle.out
963 963 48752 /tmp/res-paralle.out
[jarek:~/code/airflow] enable-mypy-as-non-failing-step(+1/-2)+ ±
```
--
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] josh-fell edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982700919
Happy to help here. Do you suspect a large number of fixes needed, and if so, should we coordinate over a Slack channel or just keep it to this 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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-987223138
@potiuk Perhaps I should hold off based on [this comment](https://github.com/apache/airflow/pull/20034#discussion_r762427587)? Or maybe just start tackling `airflow/providers/**/hooks` in the meantime? WDYT?
That comment just raises concern about not fixing `BaseOperator` first and having some trickle effect we'll have to fix again in operators.
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-989873848
> And the faster we fix the common packages, the faster the "dependent" packages will stabilize and the numbers will go down :)
I've also noticed that MyPy gives different results based on if you just check a module in isolation or the whole code base (i.e. `mypy airflow/models/dag.py` could say 0 errors, but `mypy airflow` could still list errors in dag.py. Fun)
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - #20304
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994409090
Could you post the Mypy errors you can’t figure out?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994758499
1) the mixin has no super class. super().execute() will only work because Mixin is only applied to an Operator where BaseOperator has an execute() method. We could create a custom protocol for the mixin, but I think in this case #type: ignore with appropriate comment should be good.
2) The get_db_hook() from BaseSQLOperator expects DBAPIHook to be returned. QBoleCheckHook does not extend DBAPIHook. Just extending from DBAPIHook should help.
Comment - this class hierarchy for Qubole is a mess. You can even see comment from @xinbinhuang which I very much agree with:
> # TODO(xinbinhuang): refactor to reduce levels of inheritance
But I think this is for another time (and likely someone from Qubole team to take care about it. Anyone from Qubole listening ;) ?
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995362082
Is there a way to fix this? Thanks
```
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:328: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.QUEUED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:331: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:334: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.SUCCESS
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:336: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.RUNNING
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:339: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
```
` def process_status(self, job_id: str, status: str) -> State:
"""Process status information for the JOB"""
status = status.lower()
if status == PodStatus.PENDING:
return State.QUEUED
elif status == PodStatus.FAILED:
self.log.error('Event with job id %s Failed', job_id)
return State.FAILED
elif status == PodStatus.SUCCEEDED:
self.log.info('Event with job id %s Succeeded', job_id)
return State.SUCCESS
elif status == PodStatus.RUNNING:
return State.RUNNING
else:
self.log.error('Event: Invalid state %s on job %s', status, job_id)
return State.FAILED
```
```
# These are TaskState only
NONE = None
REMOVED = TaskInstanceState.REMOVED
SCHEDULED = TaskInstanceState.SCHEDULED
QUEUED = TaskInstanceState.QUEUED
SHUTDOWN = TaskInstanceState.SHUTDOWN
RESTARTING = TaskInstanceState.RESTARTING
UP_FOR_RETRY = TaskInstanceState.UP_FOR_RETRY
UP_FOR_RESCHEDULE = TaskInstanceState.UP_FOR_RESCHEDULE
UPSTREAM_FAILED = TaskInstanceState.UPSTREAM_FAILED
SKIPPED = TaskInstanceState.SKIPPED
SENSING = TaskInstanceState.SENSING
DEFERRED = TaskInstanceState.DEFERRED
```
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999810061
#20470 should handle airflow/dag_processing but I did make some notes for review of small doubts I had.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997272693
We are getting CLOSER AND CLOSER: 277 errors to go!
--
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] josh-fell edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997046499
Just started going down from the top (ish), skipping `airflow` for now. #20390 should take care of `airflow/api/common/*` and `airflow/api_connexion/*`.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993845794
Started to look at
- airflow/providers/cncf/kubernetes
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995346953
I have two more waiting fore review: #20329 and #20327 (part of google) which should put the number down by a good 100 :)
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/alibaba - #20393
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - #20304
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - #20319
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
* airflow/providers/oracle-elasticsearch-yandex - #20344
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009387686
> Hmmm, We've got a bunch of errors like this:
>
> ```
> Missing named argument "cluster_name" for "EksClusterStateSensor"
> ```
>
> Which is not an error at runtime, as we use the `default_args` to provide this value (which makes sense --it's the point of default_args). I don't think there's really a way of fixing this really without adding a bunch of type ignores :(
Approach we followed was using .pyi files.
We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009754363
@ashb @uranusjr @jedcunningham (others): that actually reminded me that I we have not decided what to do with .pyi files. As mentioned in https://github.com/apache/airflow/issues/19891#issuecomment-1001531572
> **This is a very good point. Technically we should do it before we reease the files in our source package. I am not sure if we want to release them, but this is another question which i think will need to be answered separately - and I think we shoud have a spearate discussion on how and whether to release them:
> * we could not release them and only leave them in the repo
> * we could release them in sources or
> * we could release a separate typeshed package for those who want to use mypy https://github.com/python/typeshed
I think some of the important stub files (Context especially) we should directly include in "airflow" package. But some others (especially if we decide to leave them - for now at least - to solve some of the "default args + examples" case, we should not include). Currently we do not include any of the stub files (and we do not have a typeshed package).
We do not have a big number of stub files yet - most of them are provider's "workaround". I am quite sure we should include context.pyi - not sure about db_types/functools.
```
./airflow/utils/context.pyi
./airflow/migrations/db_types.pyi
./airflow/compat/functools.pyi
./airflow/providers/amazon/aws/operators/eks.pyi
./airflow/providers/amazon/aws/operators/s3.pyi
./airflow/providers/amazon/aws/sensors/eks.pyi
./airflow/providers/alibaba/cloud/operators/oss.pyi
./airflow/providers/microsoft/azure/operators/wasb_delete_blob.pyi
./airflow/providers/microsoft/azure/operators/cosmos.pyi
./airflow/providers/microsoft/azure/transfers/azure_blob_to_gcs.pyi
./airflow/providers/microsoft/azure/transfers/local_to_wasb.pyi
./airflow/providers/microsoft/azure/sensors/wasb.pyi
./airflow/providers/microsoft/azure/sensors/cosmos.pyi
./airflow/providers/asana/operators/asana_tasks.pyi
./airflow/providers/apache/cassandra/sensors/record.pyi
./airflow/providers/apache/cassandra/sensors/table.pyi
```
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers (we only include .py files). They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level of the exampels (outside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-986312131
> Anyone grab `airflow/providers/salesforce` yet?
Feel free! . few more merges and I am going to refresh the list to see where we are, on top of manual updatees.
--
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] Bowrna commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
Bowrna commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983462069
I will check this issue, understand it and start making contributions.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983577261
> @potiuk does it sound reasonable or shall I use pre-commit run mypy ... (which I did try but didnt get list and didnt look into it too much TBH)
I think it's better to do it inside `breeze` because there you have ALL dependencies installed in the latest "constraints-main" versions - so you are 100% sure that you have the same dependencies. The problem is that mypy will show different errors depending whcih dependencies you have installed (because for example some dependency might have added type hinting in a different version). This is why at least `./breeze` is recommended as "common" execution environment for tests (that's the main purpose of Breeze anyway).
You do not have to run pre-commits. But at least emulating what they do is useful.
Precommits under the hood do this (at least for the `airflow` test - the namespace packages are not used for chart and docs - because they got brokent if they do)
```
mypy --namespace-packages file file file ...
```
I am not 100% sure if just providing a package (as you did) will yield the same results. I think in this case `mypy` will only check "published" objectsi in `__init__.py` or something similar (but here I am not sure)
We used to do it (we converted files to packages) to overcome mypy's poor support of namespace packages in 0.770, and by seing that after ~3 weeks we have 992 errors that I see now with mypy 0.990 I am afraid part of the problem was that we used packages rather than files. But I am not at all sure of it - those might be simply because mypy 0.990 has gotten so much better since (this is quite possible as 0.770 is > 1 year old)/
You can check if it is the case now and let me know if you have different results if you try `package' vs. `module` approach)
So at least check if when you do `mypy --namespace-packages test` and `find test | xargs mypy --namespace-packaes` in ./breeze, you get the same results. My "hunch" is that they will differ. But if you enter breeze and run `find test | xargs mypy --namespace-packaes` - this is what `pre-commit` actually does.
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
Fix mypy in
- airflow/providers/apache/spark/hooks
- airflow/providers/jira
- airflow/providers/apache/cassandra/example_dags
- airflow/providers/asana
- airflow/providers/telegram
- airflow/providers/trino/hooks
- airflow/providers/postgres
https://github.com/apache/airflow/pull/20190
--
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] josh-fell edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991073962
> A better fix is
>
> ```python
> if sys.version_info >= (3, 8):
> from functools import cached_property
> else:
> from cached_property import cached_property
> ```
>
> This is the solution used in `airflow.compat.functools`, which should be used by “proper” Airflow code. But providers unfortunately cannot use it (yet). Since `cached_property` is a single-use decorator, simply ignoring the error is also considered acceptable.
@uranusjr My recent mypy fix for Azure used that marker in `airflow/providers/microsoft/azure/log/wasb_task_handler.py`. Happy to implement your suggestion instead if you feel it's best.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991929844
OK - I have quite a few google-provider related fixes in progress - I need to get them merged first because I fixed some "related" errors in few of them and I am startting to see the errors that I fixed in previous PRs. Looking forward to reviews
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1018528364
There are 12 (!) errors left :). Can I ?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015779990
I will remove all other "fake" .pyi files in separate PR.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1008431383
* 155 issues
* 27 packages
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999724742
> Question:I was on python 3.6 (when running breeze) and I now notice some inconsistencies between 3.6 and 3.8 with regards to mypy errors, especially with typing and typing_extensions, just want to check if we should be checking multiple versions.
Mypy always runs on the default version which is "3.7" at this moment (we are getting rid of Python 3.6 like - tomorrow :)
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - want to take
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - want to take
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993003911
FYI: airflow/triggers and airflow/timetables are done #20274, #20275. Although trivial fixes..
--
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] subkanthi removed a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi removed a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009387069
> We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
>
> Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
>
> Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
We fixed it by using pyi stub files.
From @potiuk
We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009754363
@ashb @uranusjr @jedcunningham (others): that actually reminded me that I we have not decided what to do with .pyi files. As mentioned in https://github.com/apache/airflow/issues/19891#issuecomment-1001531572
> **This is a very good point. Technically we should do it before we reease the files in our source package. I am not sure if we want to release them, but this is another question which i think will need to be answered separately - and I think we shoud have a spearate discussion on how and whether to release them:
> * we could not release them and only leave them in the repo
> * we could release them in sources or
> * we could release a separate typeshed package for those who want to use mypy https://github.com/python/typeshed
I think some of the important stub files (Context especially) we should directly include in "airflow" package. But some others not (especially if we decide to leave them - for now at least - to solve some of the "default args + examples" case, we should not include). Currently we do not include any of the stub files (and we do not have a typeshed package).
We do not have a big number of stub files yet - most of them are provider's "workaround". I am quite sure we should include context.pyi - not sure about db_types/functools.
```
./airflow/utils/context.pyi
./airflow/migrations/db_types.pyi
./airflow/compat/functools.pyi
./airflow/providers/amazon/aws/operators/eks.pyi
./airflow/providers/amazon/aws/operators/s3.pyi
./airflow/providers/amazon/aws/sensors/eks.pyi
./airflow/providers/alibaba/cloud/operators/oss.pyi
./airflow/providers/microsoft/azure/operators/wasb_delete_blob.pyi
./airflow/providers/microsoft/azure/operators/cosmos.pyi
./airflow/providers/microsoft/azure/transfers/azure_blob_to_gcs.pyi
./airflow/providers/microsoft/azure/transfers/local_to_wasb.pyi
./airflow/providers/microsoft/azure/sensors/wasb.pyi
./airflow/providers/microsoft/azure/sensors/cosmos.pyi
./airflow/providers/asana/operators/asana_tasks.pyi
./airflow/providers/apache/cassandra/sensors/record.pyi
./airflow/providers/apache/cassandra/sensors/table.pyi
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1004675559
I refresehd the stats. We are:
* down to 32 packages
* down to 183 errors
@kaxil if you want your team to fix some of those you need to hurry I am afraid ;)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002794694
Hey Everyone - I have done what I've been planning to do - one big PR to update Context to be typed everywhere:
1) in Providers it's `'Context'` and if TYPE_CHECKING
2) elsewher it's `Context` and direct import
I hope it will "**just work**` with all tests :D.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002794694
Hey Everyone - I have done what I've been planning to do - one big PR to update Context to be typed everywhere:
1) in Providers it's `Context` and if TYPE_CHECKING
2) elsewher it's Context and direct import
I hope it will "**just work**` with all tests :D.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002810912
And "Context" merged :)
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991353315
Just FYI - providers/google/cloud/hooks seems to be one of the biggest ones - 192 errors
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/databricks - in progress
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] khalidmammadov edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-988269481
I think we done with tests/* folder.
Starting to look into airflow/www/* if not started by anyone.
@potiuk BTW, my latest run reports:
**Found 808 errors in 212 files (checked 1820 source files)**
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991073962
> A better fix is
>
> ```python
> if sys.version_info >= (3, 8):
> from functools import cached_property
> else:
> from cached_property import cached_property
> ```
>
> This is the solution used in `airflow.compat.functools`, which should be used by “proper” Airflow code. But providers unfortunately cannot use it (yet). Since `cached_property` is a single-use decorator, simply ignoring the error is also considered acceptable.
@uranusjr My recent mypy fix for Azure used that marker in 'airflow/providers/microsoft/azure/log/wasb_task_handler.py'. Happy to implement your suggestion instead if you feel it's best.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992916546
I Updated the list of packages - we are at 897 errrors so the numbers started to go down !
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993560064
BTW. After few biggish (common) fixes I plan to re-generate the list and check where we are later today :)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994758499
1) the mixin has no super class. super().execute() will only work because Mixin is only applied to an Operator where BaseOperator has an execute() method. We could create a custom protocol for the mixin, but I think in this case #type: ignore with appropriate comment should be good.
2) The get_db_hook() from BaseSQLOperator expects DBAPIHook to be returned. QBoleCheckHook does not extend DBAPIHook. Just extending from DBAPIHook should help.
Comment - this class hierarchy for Qubole is a mess. You can even see comment from @xinbinhuang which I very much agree with:
| # TODO(xinbinhuang): refactor to reduce levels of inheritance
But I think this is for another time (and likely someone from Qubole team to take care about it. Anyone from Qubole listening ;) ?
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - #20304
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/oracle - in progress
* airflow/providers/presto - #20244
* airflow/providers/qubole - #20319
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000340991
> We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
>
> Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
>
> Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
@potiuk @josh-fell I am not sure if this method fully fixes the issue. I have applied similar changed inside airflow/providers/microsoft and when I run mypy for a folder then all good but when I run it for one file only it looks like it does not acknowledge existence of companion .pyi file in the same dir.
i.e. this is OK
`mypy --namespace-packages airflow/providers/microsoft/azure/example_dags`
but not this:
`mypy --namespace-packages airflow/providers/microsoft/azure/example_dags/example_azure_blob_to_gcs.py`
I checked `pre-commit` run approach as well it also runs on listed files and fails to detect .pyi when I replicate the command:
`pre-commit run mypy --all-files --show-diff-on-failure --color always`
Reference PR: https://github.com/apache/airflow/pull/20409
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982948431
Opened since 2015: https://github.com/python/mypy/issues/933
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992959259
I'm looking at airflow/models and tests/models
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-988269481
I think we done with tests/* folder.
Starting to look into airflow/www/security if not started by anyone.
@potiuk BTW, my latest run reports:
**Found 808 errors in 212 files (checked 1820 source files)**
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
Want to take:
* airflow/providers/databricks
* airflow/providers/apache/beam/
* airflow/providers/apache/drill/
* airflow/providers/apache/druid/
* airflow/providers/apache/sqoop/
In progress:
* airflow/providers/qubole - in progress
PR:
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/qubole - in progress
* airflow/providers/samba - in progress
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992807680
FYI @potiuk #20261 takes care of both `dev/` and `dev/provider_packages/` in the remaining packages list.
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992950925
I'm starting to look into airflow/utils if no one started
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997368387
> Anyone looking at airflow/providers/microsoft/?
Feel free!
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
Fix mypy in
- airflow/providers/apache/spark/hooks
- airflow/providers/jira
- airflow/providers/apache/cassandra/example_dags
- airflow/providers/asana
- airflow/providers/telegram
https://github.com/apache/airflow/pull/20190
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995724932
> Is there a way to fix this? Thanks
Good question - I used `State(State.QUEUED)` in some cases but I do not particularly like it - maybe others have other idea ?
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983577261
> @potiuk does it sound reasonable or shall I use pre-commit run mypy ... (which I did try but didnt get list and didnt look into it too much TBH)
I think it's better to do it inside `breeze` because there you have ALL dependencies installed in the latest "constraints-main" versions - so you are 100% sure that you have the same dependencies. The problem is that mypy will show different errors depending whcih dependencies you have installed (because for example some dependency might have added type hinting in a different version). This is why at least `./breeze` is recommended as "common" execution environment for tests and static checks like mypy and flake (that's the main purpose of Breeze anyway).
You do not have to run pre-commits. But at least emulating what they do is useful.
Precommits under the hood do this (at least for the `airflow` test - the namespace packages are not used for chart and docs - because they got brokent if they do)
```
mypy --namespace-packages file file file ...
```
I am not 100% sure if just providing a package (as you did) will yield the same results. I think in this case `mypy` will only check "published" objectsi in `__init__.py` or something similar (but here I am not sure)
We used to do it (we converted files to packages) to overcome mypy's poor support of namespace packages in 0.770, and by seing that after ~3 weeks we have 992 errors that I see now with mypy 0.990 I am afraid part of the problem was that we used packages rather than files. But I am not at all sure of it - those might be simply because mypy 0.990 has gotten so much better since (this is quite possible as 0.770 is > 1 year old)/
You can check if it is the case now and let me know if you have different results if you try `package' vs. `module` approach)
So at least check if when you do `mypy --namespace-packages test` and `find test | xargs mypy --namespace-packaes` in ./breeze, you get the same results. My "hunch" is that they will differ. But if you enter breeze and run `find test | xargs mypy --namespace-packaes` - this is what `pre-commit` actually does.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983577261
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982926067
> Ouch. I guess some/many of those are from upgrading Mypy and it adding new better checks, rather than us having broken things, right?
I am not sure. Just from the number of thos my guess is we simply missed a lot of errors via workarounding lack of namespace support (we had to convert files to packages and pass packages to mypy not files and I think it had bad side-effects).
Also I am not sure yet if mypy will detect all errors in " consistent" way with teh "parallel support from pre-commit" - it might be that when you pass all files in one run, it will detect more errors. I will check it soon to see - but it is not needed to start fixing.
We might want to switch to this non-parallel mode, though it will be even more pain for local development (but then we might finally switch to mypyd support).
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983519003
As a reference/check, this is how I get the list of issues that needs fixing in test/* folder.
I first created requirement file to install required packages (inline with latest changes into repo)
```
[venv_airflow] ~/dev/oss/airflow (main*)$ cat ../mypy_requirements.txt
mypy==0.910
types-croniter
types-docutils
types-freezegun
types-paramiko
types-protobuf
types-python-dateutil
types-python-slugify
types-pytz
types-redis
types-requests
types-setuptools
types-termcolor
types-tabulate
types-Markdown
types-PyMySQL
types-PyYAML
pip install -r ../mypy_requirements.txt
# and run below
[venv_airflow] ~/dev/oss/airflow (main*)$ mypy tests/
airflow/utils/weekday.py:62: error: Argument 1 to <set> has incompatible type "WeekDay"; expected "str"
week_day = {week_day}
^
airflow/utils/edgemodifier.py:41: error: Incompatible default for argument "label" (default has type "None", argument has type "str")
def __init__(self, label: str = None):
..........
^
tests/plugins/test_plugins_manager.py:45: error: Incompatible types in assignment (expression has type "str", variable has type Module)
importlib_metadata = 'importlib.metadata'
^
Found 426 errors in 96 files (checked 1206 source files)
```
@potiuk does it sound reasonable or shall I use pre-commit run mypy ... (which I did try but didnt get list and didnt look into it too much TBH)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-984031591
I updated the description with the "in-breeze" command and more ogranised approach in general (thanks @khalidmammadov)!
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002794694
Hey Everyone - I have done what I've been planning to do - one big PR to update Context to be typed everywhere: #20565
1) in Providers it's `'Context'` and if TYPE_CHECKING
2) elsewher it's `Context` and direct import
I hope it will "**just work**` with all tests :D.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003096591
Done. Refreshed the stats:
* down to 50 packages left (from 72)
* down to 325 errors left
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006444174
We are doing it now :D taking Amazon Provider for now -- so the following files:
- [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
- [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
- [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200) - Part (1) - https://github.com/apache/airflow/pull/20710
- [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
- [ ] ( 1) airflow/providers/amazon/aws/transfers (Me) -> https://github.com/apache/airflow/pull/20708
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015828083
> We have now come full circle with handling these pesky example DAGs.
YEP. Happens.
BTW. Experimenting and withdrawine when we see some consequences is the great part of OSS. I think we often are too reluctant to try things out and then go back if some "worries" are really founded (rather than discuss). Especially in cases that can be easily reverted :)
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009397170
> Not ideal. But it will do for now.
I'd go further than say that's not ideal and say that borderline introduces a bug that impacts our users:
Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1013900077
I am starting to look at `tests/providers/google/cloud/hooks` if no one already.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #19891:
URL: https://github.com/apache/airflow/issues/19891
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #19891:
URL: https://github.com/apache/airflow/issues/19891
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1010184538
functools.pyi is not too useful for general users IMO; it’s there because we need to support multiple Python versions; a normal setup would generally run code against one specific Python version and can simply import the real thing, not from the compat module.
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009381185
Hmmm, We've got a bunch of errors like this:
```airflow/providers/amazon/aws/example_dags/example_eks_templated.py:72: error:
Missing named argument "cluster_name" for "EksClusterStateSensor"
```
Which is not an error at runtime, as we use the `default_args` to provide this value (which makes sense --it's the point of default_args). I don't think there's really a way of fixing this really without adding a bunch of type ignores :(
--
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 issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1012560671
I just updated the description too -- to remove ones that are solved
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers. They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea.
* We could add #typeignores on global level (ouside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003782683
I have started looking into `airflow/providers/qubole`. Please let me know if anyone is looking at it already
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003551965
> I briefly looked at vertica_to_mysql.py, tmpfile is accessed here in this block
IT's because if it kinda distributed - opening using and closing of the tmpfille are happening only if "self.bulk_load" is True, so tmpfile will never be None there. - but MyPY is not analysing it that deeply.
I'd say we should refactor this method a bit. Let me attempt 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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003788264
@potiuk What's the communities view on [PEP 585](https://www.python.org/dev/peps/pep-0585/)? Looks like there will be another large refactoring needed to remove these type aliases (Dict, List etc.) as they are deprecated (in Python 3.9) in favor of native/generic types.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009774615
> A separate typestub package is only for when the package itself can't/doesn't want to release typing, so no to that option
Agree
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009398277
I've taken a quick look at a mypy plugin, and the `get_function_signature_hook` hook point does allow us to alter the type of the constructors. At least in theory.
However I have _no_ idea how to actually do that, nor how we could get the value of the default args out of the current DAG -- the mypy internals have next to zero documentaiton.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009387069
> We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
>
> Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
>
> Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
We fixed it by using pyi stub files.
From @potiuk
We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers. They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level (ouside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1020489687
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1012558770
The remaining numbers
* 110 errors
* 18 packages
Updated remaining packages.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015779598
Hey everyone here in https://github.com/apache/airflow/pull/20935 I removed `eks.pyi` file - the one we introduced to "fix" default_args. It causes many more problems than it fixes - especially when we add new classes it's super confusing.
Instead I simply added this on top of the affected examples:
```
# Ignore missing args provided by default_args
# type: ignore[call-arg]
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1018343837
Last two "biggish" https://github.com/apache/airflow/pull/21010 and https://github.com/apache/airflow/pull/20935 and we should be REALLY REALLY close.
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1018559862
> There are 12 (!) errors left :). Can I ?
OH YEAH! Fitting you get the last one IMO since you have been driving this whole effort. Although #20930 should handle `airflow/contrib/sensors/` 🙂
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/snowflake
#20212
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995362082
Is there a way to fix this? Thanks
```
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:328: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.QUEUED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:331: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:334: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.SUCCESS
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:336: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.RUNNING
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:339: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
```
` def process_status(self, job_id: str, status: str) -> State:
"""Process status information for the JOB"""
status = status.lower()
if status == PodStatus.PENDING:
return State.QUEUED
elif status == PodStatus.FAILED:
self.log.error('Event with job id %s Failed', job_id)
return State.FAILED
elif status == PodStatus.SUCCEEDED:
self.log.info('Event with job id %s Succeeded', job_id)
return State.SUCCESS
elif status == PodStatus.RUNNING:
return State.RUNNING
else:
self.log.error('Event: Invalid state %s on job %s', status, job_id)
return State.FAILED`
```
# These are TaskState only
NONE = None
REMOVED = TaskInstanceState.REMOVED
SCHEDULED = TaskInstanceState.SCHEDULED
QUEUED = TaskInstanceState.QUEUED
SHUTDOWN = TaskInstanceState.SHUTDOWN
RESTARTING = TaskInstanceState.RESTARTING
UP_FOR_RETRY = TaskInstanceState.UP_FOR_RETRY
UP_FOR_RESCHEDULE = TaskInstanceState.UP_FOR_RESCHEDULE
UPSTREAM_FAILED = TaskInstanceState.UPSTREAM_FAILED
SKIPPED = TaskInstanceState.SKIPPED
SENSING = TaskInstanceState.SENSING
DEFERRED = TaskInstanceState.DEFERRED
```
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/alibaba - #20393
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - #20304
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/oracle - #20344
* airflow/providers/presto - #20244
* airflow/providers/qubole - #20319
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997368577
> A think there are left hardest ones: microsoft, amazon, and google providers, and some `airflow/*` folders. I could also take something after the revision
Google is almost done :). Also - we know have numbers showing how many issues are left in each package :).
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000345868
> Reference PR: #20409 -- PS. I had to disable pre-commit to be able to push it (for discussion)...
Yes @khalidmammadov - it does not work if you add `example_*.pyi` - we noticed that before (and it makes sense because definition of the class is elsewhere. You should add .pyi in the package where the classs is imported from.
In #20409 you tried to change the definition in the example* and it's not going to work because stubs only work if they are next to the class they stub as I understand it
It is not perfect solution - of course, because it does not show you that the class has mandatory parameter when you use it so we will have to replace it with something better, but it will work for now.
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999993844
Is there a suggested fix for missing argument when default_args are defined?
airflow/providers/amazon/aws/example_dags have a lot of these errors.
```
airflow/providers/amazon/aws/example_dags/example_s3_bucket_tagging.py:45: error: Missing named argument "bucket_name" for
"S3CreateBucketOperator"
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
```
with DAG(
dag_id='s3_bucket_tagging_dag',
schedule_interval=None,
start_date=datetime(2021, 1, 1),
catchup=False,
default_args={"bucket_name": BUCKET_NAME},
max_active_runs=1,
tags=['example'],
) as dag:
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999993844
Is there a suggested fix for missing argument, airflow/providers/amazon/aws/example_dags have a lot of these errors.
```
airflow/providers/amazon/aws/example_dags/example_s3_bucket_tagging.py:45: error: Missing named argument "bucket_name" for
"S3CreateBucketOperator"
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
`with DAG(
dag_id='s3_bucket_tagging_dag',
schedule_interval=None,
start_date=datetime(2021, 1, 1),
catchup=False,
default_args={"bucket_name": BUCKET_NAME},
max_active_runs=1,
tags=['example'],
) as dag:
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
`
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006444174
We are doing it now :D taking Amazon Provider for now -- so the following files:
- [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
- [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
- [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200) - Part (1) - https://github.com/apache/airflow/pull/20710
- [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
- [x] ( 1) airflow/providers/amazon/aws/transfers (Me) -> https://github.com/apache/airflow/pull/20708
PRs: (#20708) (#20710) (#20713) (#20715) (#20714) (#20716) (#20717) -- #20353 is blocking some of the work
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006662748
Todays numbers (going down fast :) ):
* 172 issues
* 29 packages
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982912000
Oof. Let the fun begin!
Naturally, I'll sign up for all of the `*/example_dags`. And I'll take the Azure ones too if you don't mind.
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-987270240
@khalidmammadov You seem to be cruising on the `tests/*` set but let me know if there are any you'd like some help on or take off your plate until we get a green light on cranking through the providers.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-987274517
@josh-fell - start from the end :D
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-985948458
I took a crack at `airflow/decorators` but 1 error is eluding me (see #20034).
Anyone grab `airflow/providers/salesforce` yet?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992457354
I added `mypy` label (temporarily) to keep track of all the opened MyPy PRs https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Amypy
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994116156
Hi team.
I'm a little bit stuck with the Qubole operator because of the `[attr-defined]` error in Mixin.
Could you share some articles and examples of how you handle that?
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995362082
Is there a way to fix this? Thanks
```
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:328: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.QUEUED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:331: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:334: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.SUCCESS
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:336: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.RUNNING
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:339: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
```
def process_status(self, job_id: str, status: str) -> State:
"""Process status information for the JOB"""
status = status.lower()
if status == PodStatus.PENDING:
return State.QUEUED
elif status == PodStatus.FAILED:
self.log.error('Event with job id %s Failed', job_id)
return State.FAILED
elif status == PodStatus.SUCCEEDED:
self.log.info('Event with job id %s Succeeded', job_id)
return State.SUCCESS
elif status == PodStatus.RUNNING:
return State.RUNNING
else:
self.log.error('Event: Invalid state %s on job %s', status, job_id)
return State.FAILED
```
```
# These are TaskState only
NONE = None
REMOVED = TaskInstanceState.REMOVED
SCHEDULED = TaskInstanceState.SCHEDULED
QUEUED = TaskInstanceState.QUEUED
SHUTDOWN = TaskInstanceState.SHUTDOWN
RESTARTING = TaskInstanceState.RESTARTING
UP_FOR_RETRY = TaskInstanceState.UP_FOR_RETRY
UP_FOR_RESCHEDULE = TaskInstanceState.UP_FOR_RESCHEDULE
UPSTREAM_FAILED = TaskInstanceState.UPSTREAM_FAILED
SKIPPED = TaskInstanceState.SKIPPED
SENSING = TaskInstanceState.SENSING
DEFERRED = TaskInstanceState.DEFERRED
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992809468
Yeah. Good point @josh-fell . Marked.
But also I will keep on re-running the script every few days as we merge more fixes so hopefully all the "overloooked" packages will disappear "automatically" :)
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994633952
@uranusjr
`mypy --show-error-codes --namespace-packages airflow/providers/qubole/operators/qubole_check.py`
<img width="1636" alt="Screenshot 2021-12-15 at 12 23 49" src="https://user-images.githubusercontent.com/10523218/146169180-3982f383-c840-4361-9965-0c89a5a1f2e0.png">
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997306200
A think there are left hardest ones: microsoft, amazon, and google providers, and some `airflow/*` folders.
I could also take something after the revision
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997294763
Anyone looking at airflow/providers/microsoft/?
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997046499
Just started going down from the top (ish), skipping `airflow` for now. #20390 should take care of `airflow/api/common/*` and `airflow/api_connexion/*`.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997954100
I'll take the remaining:
```
airflow/hooks
airflow/jobs
airflow/macros
airflow/operators
airflow/example_dags
```
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000900089
Question , looking at this PR, is the convention to not include the ASF source header for pyi files
https://github.com/apache/airflow/pull/20422
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003432071
I briefly looked at vertica_to_mysql.py, tmpfile is accessed here in this block
```
cursor.execute(
f"LOAD DATA LOCAL INFILE '{tmpfile.name}' "
f"INTO TABLE {self.mysql_table} "
f"LINES TERMINATED BY '\r\n' ({', '.join(selected_columns)})"
)
```
which is outside of the with block, not sure if this is a bug or maybe Im missing something.
``` with NamedTemporaryFile("w") as tmpfile:```
```
count = 0
with closing(vertica.get_conn()) as conn:
with closing(conn.cursor()) as cursor:
cursor.execute(self.sql)
selected_columns = [d.name for d in cursor.description]
if self.bulk_load:
with NamedTemporaryFile("w") as tmpfile:
self.log.info("Selecting rows from Vertica to local file %s...", tmpfile.name)
self.log.info(self.sql)
csv_writer = csv.writer(tmpfile, delimiter='\t', encoding='utf-8')
for row in cursor.iterate():
csv_writer.writerow(row)
count += 1
tmpfile.flush()
else:
self.log.info("Selecting rows from Vertica...")
self.log.info(self.sql)
result = cursor.fetchall()
count = len(result)
self.log.info("Selected rows from Vertica %s", count)
if self.mysql_preoperator:
self.log.info("Running MySQL preoperator...")
mysql.run(self.mysql_preoperator)
try:
if self.bulk_load:
self.log.info("Bulk inserting rows into MySQL...")
with closing(mysql.get_conn()) as conn:
with closing(conn.cursor()) as cursor:
cursor.execute(
f"LOAD DATA LOCAL INFILE '{tmpfile.name}' "
f"INTO TABLE {self.mysql_table} "
f"LINES TERMINATED BY '\r\n' ({', '.join(selected_columns)})"
)
conn.commit()
tmpfile.close()
else:
self.log.info("Inserting rows into MySQL...")
mysql.insert_rows(table=self.mysql_table, rows=result, target_fields=selected_columns)
self.log.info("Inserted rows into MySQL %s", count)
except (MySQLdb.Error, MySQLdb.Warning):
self.log.info("Inserted rows into MySQL 0")
raise
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1001531572
> Question , looking at this PR, is the convention to not include the ASF source header for pyi files
This is a very good point. Technically we should do it before we reease the files in our source package. I am not sure if we want to release them, but this is another question which i think will need to be answered separately - and I think we shoud have a spearate discussion on how and whether to release them:
* we could not release them and only leave them in the repo
* we could release them in sources or
* we could release a separate typeshed package for those who want to use mypy https://github.com/python/typeshed
But regardless of the discussion - those fiels are manualy modified so technically speaking they should be licenced. I will update the pre-commit to include generation of headers also in those files. So even if we will not release them, it makes no harrm to add licence header.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002805531
> I hope it will "**just work**` with all tests :D.
Looks like it does :).
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002966435
Woooooohooo.. After we finally re-enable MyPy - there will be next step. New, rather interesting MyPy release:
https://mypy-lang.blogspot.com/2021/12/mypy-0930-released.html?spref=tw
Some realy cool features there that we could ue - explicit type aliases and NotRequired TypeDict elements :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991224343
> Yes please @josh-fell , we can't use `airflow.compat.functools` in providers, that module only got added in 2.2.0, so if we use that in providers it would need a min airflow version of 2.2.0.
And @josh-fell - in case you asked - if you try to use the functools, the CI would have failed in the "install providers on 2.1" CI test :).
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
Fix mypy in
- airflow/providers/apache/spark/hooks
- airflow/providers/jira
- airflow/providers/apache/cassandra/example_dags
- airflow/providers/asana
- airflow/providers/telegram
- airflow/providers/trino/hooks
https://github.com/apache/airflow/pull/20190
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991353315
Just FYI - providers/google/cloud/hooks seems to be one of the biggest ones - 192 errors
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/snowflake
#20212
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991224343
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983460610
Perfect!
--
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] josh-fell edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982912000
Oof. Let the fun begin!
Naturally, I'll sign up for all of the `*/example_dags`. And I'll take the Azure ones too if you don't mind as my first pass.
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982921547
Ouch. I guess some/many of those are from upgrading Mypy and it adding new better checks, rather than us having broken things, right?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982914984
But we need to merge https://github.com/apache/airflow/pull/19890 first
--
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 issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006444174
We are doing it now :D taking Amazon Provider for now -- so the following files:
- [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
- [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
- [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200)
- [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
- [ ] ( 1) airflow/providers/amazon/aws/transfers (Me)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1008431383
* 155 issue
* 27 packages
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009757315
context.pyi - absolutely, as that one is _mostly_ for users than for Airflow itself.
db_types.pyi - not needed to be shipped, it's mostly for migrations, so developers
functools.pyi - 50/50. Not _designed_ for users, but someone could use it so I think I'd say yes to ship that one.
Providers I have no context on, but going on this discussion exclude then from the dists.
--
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 issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1012562669
I had just updated the list in the description , @potiuk looks like your change reverted it.. some of the ones are already solved like `imap` and the others. Check "Edit History" of the pr description
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015818650
We have now come full circle with handling these pesky example DAGs.
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1012324015
Is there anything left to do here?
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-989536477
I started looking into `dev`. Let me know if someone else has already though.
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
Fix mypy in
- airflow/providers/apache/spark/hooks
- airflow/providers/jira
- airflow/providers/apache/cassandra/example_dags
- airflow/providers/asana
https://github.com/apache/airflow/pull/20190
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/snowflake #20212
* airflow/providers/qubole - in progress
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/presto - in progress
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-989855072
All right I regenerated the current list of packages, and it went a bit down :).
Not every package we "cleaned up" is gone from the list because as we clean-up and add Types in the "common" parts - new errors might appear in the packages that are using them.
From what i checked in "Breeze" we have still 952 errors - but I think it should not discourage us. In the initial period I believe it will fluctuate up and down because of the above phenomena, but I hope very soon the numbers will start going down quickly!
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/snowflake #20212
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/qubole - in progress
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-994073297
Updated the list - we are down to 758 errors (was 897).
--
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 issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003405046
If you can, please leave amazon and probably a couple of providers for us, some of my team members wants to work on and contribute with their First PRs next Thursday
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006444174
We are doing it now :D taking Amazon Provider for now -- so the following files:
- [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
- [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
- [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200) - Part (1) - https://github.com/apache/airflow/pull/20710
- [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
- [x] ( 1) airflow/providers/amazon/aws/transfers (Me) -> https://github.com/apache/airflow/pull/20708
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1020489687
ALLRIGHT:
:crossed_fingers: everyone: https://github.com/apache/airflow/pull/21020
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997954100
I'll take the remaining "airflow-core" (some of those re-appeared):
```
airflow/hooks
airflow/jobs
airflow/macros
airflow/operators
airflow/example_dags
```
--
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] kazanzhy commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995649240
I was very surprised when saw an error in `tests/providers/snowflake`. I was sure that it's done.
Unfortunately, I fixed it in only one place and missed two ones.
Here is the PR https://github.com/apache/airflow/pull/20212/files
@khalidmammadov
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982951040
BTW. @josh-fell and others - I already have quite a number of fixes - semi-automated - (but not a lot of them are verified in https://github.com/apache/airflow/pull/19334 - so let me take a look and maybe some packages/parts will be cleanly applicable and we can get rid of many issues - but I want to merge this one first and the apply what I have there package by package (or maybe group of packages0
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982700919
Love this! Happy to help here. Do you suspect a large number of fixes needed, and if so, should we coordinate over a Slack channel or just keep it to this 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] potiuk commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-984021635
Sounds GOOD. So @ashb answering your earlier question from https://github.com/apache/airflow/issues/19891#issuecomment-982921547 - it looks like all the 900+ errors ARE coming from migrating to the new mypy.
We should have done that a loooong time ago :).
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983380190
I just read a few PRs on this, and the general impression I’m getting is we are using `type: ignore` way too much. I feel we should try a bit harder to fix the actual typing errors instead of liberally slapping on ignores everywhere.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983414543
cc: @khalidmammadov
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers. They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level of the exampels (outside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999993844
Is there a suggested fix for missing argument, airflow/providers/amazon/aws/example_dags have a lot of these errors.
```
airflow/providers/amazon/aws/example_dags/example_s3_bucket_tagging.py:45: error: Missing named argument "bucket_name" for
"S3CreateBucketOperator"
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
```
with DAG(
dag_id='s3_bucket_tagging_dag',
schedule_interval=None,
start_date=datetime(2021, 1, 1),
catchup=False,
default_args={"bucket_name": BUCKET_NAME},
max_active_runs=1,
tags=['example'],
) as dag:
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999757990
I will take google (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] potiuk commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002805531
> I hope it will "**just work**` with all tests :D.
Looks like it is :).
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982926067
> Ouch. I guess some/many of those are from upgrading Mypy and it adding new better checks, rather than us having broken things, right?
I am not sure. Just from the number of those my guess is we simply missed a lot of errors via workarounding lack of namespace support (we had to convert files to packages and pass packages to mypy not files and I think it had bad side-effects).
Also I am not sure yet if mypy will detect all errors in " consistent" way with the "parallel support from pre-commit" - it might be that when you pass all files in one run, it will detect more errors. I will check it soon to see - but it is not needed to start fixing.
We might want to switch to this non-parallel mode, though it will be even more pain for local development (but then we might finally switch to mypyd support).
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003074556
> Is anyone looking at this - airflow/providers/google/cloud/operators/stackdriver.py?
Feel free. but wait until I merge #20571 as there might be overlapping changes (should be in ~ 10 minutes after all tests run).
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015504504
Started looking at `airflow/contrib/sensors` but let me know if anyone else has done so already.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015794655
Removing all fakes here: https://github.com/apache/airflow/pull/20936
Even if we loose some nice things (automated verification of "REALLY" missing arguements), this is far better than getting some errors when the .pyi files are not updated.
--
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] josh-fell edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992746919
I can pickup `docs/exts/*` and `scripts/in_container` next.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995219401
- Looking at airflow/providers/amazon/aws/hooks
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - want to take
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - #20301
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - #20270
* airflow/providers/apache/sqoop - #20304
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - #20319
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - want to take
* airflow/providers/apache/drill - #20268
* airflow/providers/apache/druid - want to take
* airflow/providers/apache/sqoop - want to take
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992952058
I already have a PR for that @khalidmammadov #19914
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993527985
Anyone looking at airflow/serialization/, airflow/sensors/, airflow/security/ and airflow/secrets? (PS.There is only one small issue in the security and secrets folder per each)
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/snowflake #20212
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991084818
> > A better fix is
> > ```python
> > if sys.version_info >= (3, 8):
> > from functools import cached_property
> > else:
> > from cached_property import cached_property
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > This is the solution used in `airflow.compat.functools`, which should be used by “proper” Airflow code. But providers unfortunately cannot use it (yet). Since `cached_property` is a single-use decorator, simply ignoring the error is also considered acceptable.
>
> @uranusjr My recent mypy fix for Azure used that marker in `airflow/providers/microsoft/azure/log/wasb_task_handler.py`. Happy to implement your suggestion instead if you feel it's best.
Yes please @josh-fell , we can't use `airflow.compat.functools` in providers, that module only got added in 2.2.0, so if we use that in providers it would need a min airflow version of 2.2.0.
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
Want to take:
* airflow/providers/databricks
* airflow/providers/apache/beam/
* airflow/providers/apache/drill/
* airflow/providers/apache/druid/
* airflow/providers/apache/sqoop/
In progress:
* airflow/providers/qubole - in progress
PR:
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/databricks - want to take
* airflow/providers/apache/beam - want to take
* airflow/providers/apache/drill - want to take
* airflow/providers/apache/druid - want to take
* airflow/providers/apache/sqoop - want to take
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] kazanzhy removed a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy removed a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995649240
I was very surprised when saw an error in `tests/providers/snowflake`. I was sure that it's done.
Unfortunately, I fixed it in only one place and missed two ones.
Here is the PR https://github.com/apache/airflow/pull/20212/files
@khalidmammadov
--
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] khalidmammadov edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000340991
> We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
>
> Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
>
> Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
@potiuk @josh-fell I am not sure if this method fully fixes the issue. I have applied similar changed inside airflow/providers/microsoft and when I run mypy for a folder then all good but when I run it for one file only it looks like it does not acknowledge existence of companion .pyi file in the same dir.
i.e. this is OK
`mypy --namespace-packages airflow/providers/microsoft/azure/example_dags`
but not this:
`mypy --namespace-packages airflow/providers/microsoft/azure/example_dags/example_azure_blob_to_gcs.py`
I checked `pre-commit` run approach as well it also runs on listed files and fails to detect .pyi when I replicate the command:
`pre-commit run mypy --all-files --show-diff-on-failure --color always`
Reference PR: https://github.com/apache/airflow/pull/20409 -- PS. I had to disable pre-commit to be able to push it (for discussion)...
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999993844
Is there a suggested fix for missing argument, airflow/providers/amazon/aws/example_dags have a lot of these errors.
```
airflow/providers/amazon/aws/example_dags/example_s3_bucket_tagging.py:45: error: Missing named argument "bucket_name" for
"S3CreateBucketOperator"
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
```
`
with DAG(
dag_id='s3_bucket_tagging_dag',
schedule_interval=None,
start_date=datetime(2021, 1, 1),
catchup=False,
default_args={"bucket_name": BUCKET_NAME},
max_active_runs=1,
tags=['example'],
) as dag:
create_bucket = S3CreateBucketOperator(task_id='s3_bucket_tagging_dag_create', region_name='us-east-1')
`
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000121536
We fixed similar problems by introducing .pyi definition for operator which has the field marked as optional/having default:
Example: https://github.com/apache/airflow/pull/20422/files#diff-b203bbb3091d2f66173522a470009ba78c142564717c2168ad39d39ff7ed9a11R1
Not ideal. But it will do for now. We will likely have to implement a custom mypy plugin to allow passing non-optional parameters by DAG's default_args and we can do that later.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991749451
@subkanthi - I will take the google one :scream:
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999719624
Question:I was on python 3.6 (when running breeze) and I now notice some inconsistencies between 3.6 and 3.8 with regards to mypy errors, especially with typing and typing_extensions, just want to check if we should be checking multiple versions.
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002299683
Anyone looking at `tests/providers/amazon`?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1001535019
cc: @ashb @uranusjr @kaxil @jedcunningham -> I think we we need to make the decision on how/whether we release those files before 2.3. I'd say releasing all .pyi files together with airflow makes most sense.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002966435
Woooooohooo.. After we finally re-enable MyPy - there will be next step. New, rather interesting MyPy release:
https://mypy-lang.blogspot.com/2021/12/mypy-0930-released.html?spref=tw
Some realy cool features there that we could use - explicit type aliases and NotRequired TypeDict elements :)
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003073276
Is anyone looking at this - airflow/providers/google/cloud/operators/stackdriver.py?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003431463
* down to 38 packages
* down to 218 errrors
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982885386
Yeah we have about 800 issues :( . I think what I might do is to generate a list of packages and we can subscribe to them here in comments and check them when they are done.
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-984016038
@potiuk I run below inside ./breeze. Looks like does not make much difference.
mypy --namespace-packages tests/
# Reported:
# Found 145 errors in 32 files (checked 1208 source files)
find tests/ -name "*.py" | xargs mypy --namespace-packages
# Reported
# Found 145 errors in 32 files (checked 1208 source files)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982908097
There you are @josh-fell - we have 992 unique issues
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983577261
> @potiuk does it sound reasonable or shall I use pre-commit run mypy ... (which I did try but didnt get list and didnt look into it too much TBH)
I think it's better to do it inside `breeze` because there you have ALL dependencies installed in the latest "constraints-main" versions - so you are 100% sure that you have the same dependencies, and the problem is that mypy will show different errors depending whcih dependencies you have installed (because for example some dependency might have added type hinting in a different version). This is why at least `./breeze` is recommended as "common" execution environment for tests (that's the main purpose of Breeze anyway).
You do not have to run pre-commits. But at least emulating what they do is useful.
Precommits under the hood do this (at least for the `airflow` test - the namespace packages are not used for chart and docs - because they got brokent if they do)
```
mypy --namespace-packages file file file ...
```
I am not 100% sure if just providing a package (as you did) will yield the same results. I think in this case `mypy` will only check "published" objectsi in `__init__.py` or something similar (but here I am not sure)
We used to do it (we converted files to packages) to overcome mypy's poor support of namespace packages in 0.770, and by seing that after ~3 weeks we have 992 errors that I see now with mypy 0.990 I am afraid part of the problem was that we used packages rather than files. But I am not at all sure of it - those might be simply because mypy 0.990 has gotten so much better since (this is quite possible as 0.770 is > 1 year old)/
You can check if it is the case now and let me know if you have different results if you try `package' vs. `module` approach)
So at least check if when you do `mypy --namespace-packages test` and `find test | xargs mypy --namespace-packaes` in ./breeze, you get the same results. My "hunch" is that they will differ. But if you enter breeze and run `find test | xargs mypy --namespace-packaes` - this is what `pre-commit` actually does.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers. They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
But what is really lying to our users are the default args in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea.
* We could add #typeignores on global level (ouside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers (we only include .py files). They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add those .pyi files, but adding # typeignores is I think even worse). And yeah - agree mypy's documentation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level of the exampels (outside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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 removed a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil removed a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1012560671
I just updated the description too -- to remove ones that are solved
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003829748
The typed aliases are intentionally given a very long deprecation period and can be modified in a vastly more gradual pace. I would recommend one of the followings:
1. Keep using alises in `typing` and only start using PEP 585 definitions when Airflow drops 3.8 support (which gives the transition around two years).
2. Move the non-alised definitions when there is an official decision on PEP 649 (which allows you to safely use `from __future__ import annotation`).
3. Convert all alised annotations to stringified stdlib definitions (e.g. change `Dict[str, List[str]]` to `"dict[str, list[str]]"`; note the surrounding quotes). This works right now and does not need any future work to finish the transition, but is obviously not as pretty.
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1004030591
Anyone looking at `airflow/dag_processing/processor.py`?
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-987288549
@josh-fell Sure things, I have started from the top of tests and now on and doing tests/providers. As @potiuk mentioned, you can start from other end (www) and so we can get it done soon. Thanks
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982912739
I will start with "airflow" part :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983399390
Those PRs were just "Quick" extract of what I've done before really quickly - but I agreee we should rather refactore some of the "core" code because it is often done "badly" using shortcuts and very loose approach to types (reusing same variables for different types freely for example).
I hope more peopel will join and it will take just a short while to complete .
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-996188372
Also looking at providers/amazon/aws/operators as the fixes in /aws/hooks affect operators
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995300974
Today's lucky numbers: 758 -> 614. Going down fast now. List updated.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-987228712
Likely. Possibly reworking tests is better idea . Some overlap is bound to happen, but I am not sure if there are just a few things that we will have to redo, it's a big problem. We have 900 errors to fix :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997920529
Taking on "apache" providers
--
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] khalidmammadov edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-984016038
@potiuk I run below inside ./breeze. Looks like does not make much difference.
```
mypy --namespace-packages tests/
# Reported:
# Found 145 errors in 32 files (checked 1208 source files)
find tests/ -name "*.py" | xargs mypy --namespace-packages
# Reported
# Found 145 errors in 32 files (checked 1208 source files)
```
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983577261
> @potiuk does it sound reasonable or shall I use pre-commit run mypy ... (which I did try but didnt get list and didnt look into it too much TBH)
I think it's better to do it inside `breeze` because there you have ALL dependencies installed in the latest "constraints-main" versions - so you are 100% sure that you have the same dependencies. The problem is that mypy will show different errors depending whcih dependencies you have installed (because for example some dependency might have added type hinting in a different version). This is why at least `./breeze` is recommended as "common" execution environment for tests and static checks like mypy and flake (that's the main purpose of Breeze anyway).
You do not have to run pre-commits. But at least emulating what they do is useful.
Precommits under the hood do this (at least for the `airflow` test - the namespace packages are not used for chart and docs - because they got brokent if they do)
```
mypy --namespace-packages file file file ...
```
I am not 100% sure if just providing a package (as you did) will yield the same results. I think in this case `mypy` will only check "published" objectsi in `__init__.py` or something similar (but here I am not sure)
We used to do it (we converted files to packages) to overcome mypy's poor support of namespace packages in 0.770, and by seing that after ~3 weeks we have 992 errors that I see now with mypy 0.990 I am afraid part of the problem was that we used packages rather than files. But I am not at all sure of it - those might be simply because mypy 0.990 has gotten so much better since (this is quite possible as 0.770 is > 1 year old)/
You can check if it is the case now and let me know if you have different results if you try `package` vs. `modules` approach)
So at least check if when you do `mypy --namespace-packages test` and `find test | xargs mypy --namespace-packages` in ./breeze, you get the same results. My "hunch" is that they will differ. But if you enter breeze and run `find test | xargs mypy --namespace-packages` - this is what `pre-commit` actually does.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999732922
Thanks, so does that mean we don't have to worry about errors in 3.8 and just focus on 3.7.
in the case of typing/typing_extensions, 3.6 and 3.7 are the same and 3.8 behaves differently.
```
try:
# Literal, Protocol and TypedDict are only added to typing module starting from
# python 3.8 we can safely remove this shim import after Airflow drops
# support for <3.8
from typing import Literal, Protocol, TypedDict, runtime_checkable # type: ignore
except ImportError:
print("Import error")
from typing_extensions import Literal, Protocol, TypedDict, runtime_checkable # type: ignore # noqa
```
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999757626
348 -> looks like a hydra. You cut some heads, new ones grow :).
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000345868
> Reference PR: #20409 -- PS. I had to disable pre-commit to be able to push it (for discussion)...
Yes @khalidmammadov - it does not work if you add `example_*.pyi` - we noticed that before (and it makes sense because definition of the class is elsewhere. You should add .pyi in the package where the classs is imported from (look at my example).
In #20409 you tried to change the definition in the example* and it's not going to work because stubs only work if they are next to the class they stub as I understand it
It is not perfect solution - of course, because it does not show you that the class has mandatory parameter when you use it so we will have to replace it with something better, but it will work for now.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000345868
> Reference PR: #20409 -- PS. I had to disable pre-commit to be able to push it (for discussion)...
Yes @khalidmammadov - it does not work if you add example_*.pyi - we noticed that before (and it makes sense because definition of the class is elsewhere. You should add .pyi in the package where the classs is imported from.
In #20409 you tried to change the definition in the example* and it's not going to work because stubs only work if they are next to the class they stub as I understand it
It is not perfect solution - of course, because it does not show you that the class has mandatory parameter when you use it so we will have to replace it with something better, but it will work for now.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003958947
I concur with @uranusjr assesment. Even in PEP 585 it is written:
> Usefulness of this syntax before PEP 585 is limited as external tooling like Mypy does not recognize standard collections as generic. Moreover, certain features of typing like type aliases or casting require putting types outside of annotations, in runtime context. While these are relatively less common than type annotations, it's important to allow using the same type syntax in all contexts. This is why starting with Python 3.9, the following collections become generic using __class_getitem__() to parameterize contained types:
I think this will be a good exercise to make when we will prepare to drop Python 3.8 and for now using `typing` is best (ar actually when MyPy will start reporting that as a warning).
Actually (side comment), I do not really like the `abc` in Collections :). This ship has sailed already. But until the original collection.* is there this adds even more confusion (should collections.Sequence also be used as generic as well as collections.abc.Sequence ?) . The old collections.* imports will be removed in 3.9. But tt did not happen. They were finally dropped in 3.10, I believe. And only then we will have somewhat sane collections imports :). so we have indeed years until this becomes even a warning.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003390340
Let me (again) take the remaining providers/google ones :)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1004675559
I refreshed the stats. We are:
* down to 32 packages
* down to 183 errors
@kaxil if you want your team to fix some of those you need to hurry I am afraid ;)
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1006444174
We are doing it now :D taking Amazon Provider for now -- so the following files:
- [ ] ( 46) airflow/providers/amazon/aws/example_dags (@phanikumv)
- [ ] ( 14) airflow/providers/amazon/aws/hooks (@bharanidharan14)
- [ ] ( 16) airflow/providers/amazon/aws/operators (@sunank200)
- [ ] ( 9) airflow/providers/amazon/aws/sensors (@rajaths010494)
- [ ] ( 1) airflow/providers/amazon/aws/transfers (Me) -> https://github.com/apache/airflow/pull/20708
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015828083
> We have now come full circle with handling these pesky example DAGs.
YEP
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991095857
> > > A better fix is
> > > ```python
> > > if sys.version_info >= (3, 8):
> > > from functools import cached_property
> > > else:
> > > from cached_property import cached_property
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > This is the solution used in `airflow.compat.functools`, which should be used by “proper” Airflow code. But providers unfortunately cannot use it (yet). Since `cached_property` is a single-use decorator, simply ignoring the error is also considered acceptable.
> >
> >
> > @uranusjr My recent mypy fix for Azure used that marker in `airflow/providers/microsoft/azure/log/wasb_task_handler.py`. Happy to implement your suggestion instead if you feel it's best.
>
> Yes please @josh-fell , we can't use `airflow.compat.functools` in providers, that module only got added in 2.2.0, so if we use that in providers it would need a min airflow version of 2.2.0.
On it. Thanks @ashb. I'll add the version check instead of using the `ignore[attr-defined]` comment that's there now.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991749653
Just one corretion - it looks like 294 errors when I run 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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/apache/beam - want to take
* airflow/providers/apache/drill - want to take
* airflow/providers/apache/druid - want to take
* airflow/providers/apache/sqoop - want to take
* airflow/providers/databricks - #20265
* airflow/providers/http - #20246
* airflow/providers/presto - #20244
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992959692
Looking at airflow/providers/airbyte
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/qubole - in progress
* airflow/providers/samba - in progress
* airflow/providers/sftp - in progress
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993559258
> Anyone looking at airflow/serialization/, airflow/sensors/, airflow/security/ and airflow/secrets? (PS.There is only one small issue in the security and secrets folder per each)
Go ahead.
--
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] subkanthi commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-993591119
- airflow/providers/exasol/hooks
- airflow/providers/facebook/ads/hooks
https://github.com/apache/airflow/pull/20291
--
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] subkanthi edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-990603297
Fix mypy in
- airflow/providers/apache/spark/hooks
- airflow/providers/jira
- airflow/providers/apache/cassandra/example_dags
- airflow/providers/asana
- airflow/providers/telegram
- airflow/providers/trino/hooks
- airflow/providers/postgres
- airflow/providers/slack
https://github.com/apache/airflow/pull/20190
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/qubole - in progress
* airflow/providers/snowflake #20212
* airflow/providers/ssh - in progress
* airflow/providers/tableau - #20240
--
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] josh-fell commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
josh-fell commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-992746919
I can pickup `docs/ext/docs_build` and `scripts/in_container` next.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-989856145
And the faster we fix the common packages, the faster the "dependent" packages will stabilize and the numbers will go down :)
--
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] kazanzhy edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
kazanzhy edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-991367061
Fix mypy in
* airflow/providers/qubole - in progress
* airflow/providers/samba - #20243
* airflow/providers/sftp - #20242
* airflow/providers/snowflake #20212
* airflow/providers/ssh - #20241
* airflow/providers/tableau - #20240
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-996263027
I think I have all necessary changes for Google : #20358, #20329, #20234
--
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] subkanthi removed a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
subkanthi removed a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-995362082
Is there a way to fix this? Thanks
```
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:328: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.QUEUED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:331: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:334: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.SUCCESS
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:336: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.RUNNING
^
airflow/providers/cncf/kubernetes/utils/pod_launcher.py:339: error: Incompatible return value type (got "TaskInstanceState", expected "State")
return State.FAILED
```
def process_status(self, job_id: str, status: str) -> State:
"""Process status information for the JOB"""
status = status.lower()
if status == PodStatus.PENDING:
return State.QUEUED
elif status == PodStatus.FAILED:
self.log.error('Event with job id %s Failed', job_id)
return State.FAILED
elif status == PodStatus.SUCCEEDED:
self.log.info('Event with job id %s Succeeded', job_id)
return State.SUCCESS
elif status == PodStatus.RUNNING:
return State.RUNNING
else:
self.log.error('Event: Invalid state %s on job %s', status, job_id)
return State.FAILED
```
```
# These are TaskState only
NONE = None
REMOVED = TaskInstanceState.REMOVED
SCHEDULED = TaskInstanceState.SCHEDULED
QUEUED = TaskInstanceState.QUEUED
SHUTDOWN = TaskInstanceState.SHUTDOWN
RESTARTING = TaskInstanceState.RESTARTING
UP_FOR_RETRY = TaskInstanceState.UP_FOR_RETRY
UP_FOR_RESCHEDULE = TaskInstanceState.UP_FOR_RESCHEDULE
UPSTREAM_FAILED = TaskInstanceState.UPSTREAM_FAILED
SKIPPED = TaskInstanceState.SKIPPED
SENSING = TaskInstanceState.SENSING
DEFERRED = TaskInstanceState.DEFERRED
```
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-997272693
We are getting CLOSER AND CLOSER!
**277** errors to go!
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-999756964
> Thanks, so does that mean we don't have to worry about errors in 3.8 and just focus on 3.7.
Yeah. 3.8 differences will be few and far between. Not worth to check 3.7, 3.8. 3.9 seperately
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1000351524
> > Reference PR: #20409 -- PS. I had to disable pre-commit to be able to push it (for discussion)...
>
> Yes @khalidmammadov - it does not work if you add `example_*.pyi` - we noticed that before (and it makes sense because definition of the class is elsewhere. You should add .pyi in the package where the classs is imported from (look at my example). In #20409 you tried to change the definition in the example* and it's not going to work because stubs only work if they are next to the class they stub as I understand it
>
> It is not perfect solution - of course, because it does not show you that the class has mandatory parameter when you use it so we will have to replace it with something better, but it will work for now.
I see what you mean, working on 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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983383209
> I just read a few PRs on this, and the general impression I’m getting is we are using `type: ignore` way too much. I feel we should try a bit harder to fix the actual typing errors instead of liberally slapping on ignores everywhere.
Agree.
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-982912739
I will start with "airflow/ core subdirs ":)
```
airflow/executors
airflow/hooks
airflow/jobs
airflow/kubernetes
airflow/macros
airflow/models
airflow/operators
```
--
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] khalidmammadov commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-983432665
I can take a look to tests/* folder if it's ok?
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1004033300
> Anyone looking at `airflow/dag_processing/processor.py`?
Feel free!
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1003407151
> If you can, please leave amazon and probably a couple of providers for us, some of my team members wants to work on and contribute with their First PRs next Thursday
Sure just wanted to finally(!) kill Google one :): https://github.com/apache/airflow/pull/20611
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1002336697
> I fixed mypy errors in airflow/decorators and then realized it was a dup effort. I'd be happy to help if there is any work not taken by anyone yet.
See at the list in the issue - it contains a number of unresolved issues when we last checked (and check if someone has not signed up for it in the thread above). I hope to merge 3-4 more prs tomorrow and will re-run it to refresh the numbers.
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-988425025
cool. I will refresh the list from scratch tomorrow :).
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015828083
> We have now come full circle with handling these pesky example DAGs.
YEP. Happens.
BTW. Experimenting and withdrawing when we see some consequences is the great part of OSS. I think we often are too reluctant to try things out and then go back if some "worries" are really founded (rather than discuss). Especially in cases that can be easily reverted :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1020576535
Thanks everyone!
MyPY is re-enabled now. Let's keep our watchful eyes over the next few days to not merge changes that are not rebased to latest main (and quickly fix those that cause main failures it it happens).
The next one when we upgrade to new MyPY release I think :)
--
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 #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1015633740
* 108 errors left
--
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 edited a comment on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009429873
> Those `.pyi` files get shipped with the providers, and we're now lying about what arguments are required for the sake of which example args we happen to use in the example dags?
Not really. The .pyi files are not included in released providers (we only include .py files). They only impact 'example_dags" really - because those are the only "users" of the operators in airflow codebase.
And what is really lying to our users are the `default_args` in this case,
And yea, I also looked at the mypy for that (I actually hate we have to add those .pyi files, but adding #tpeignores is I think even worse). And ye agree it's docuementation for plugins is non-existing.
I think it's 'least' of the many evils we can have there
* We could add #typeignores but they would make it part of our examples. I think very, very few of our users would use MyPY to verify their dags and adding those #typeignores in examples is generally bad idea because of noise (even if technically having them in examples is what would people end up with if they do use mypy).
* We could add #typeignores on global level of the exampels (outside of the exampleinclude) but this way we won't be really properly testing the examples (and it's useful to run MyPy on them too
But if you come up with a better idea - I am all ears :)
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009376482
I went on a bit of a bash and got all the ones from Airflow "core" - #20795
--
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] ashb commented on issue #19891: Re-enable MyPy
Posted by GitBox <gi...@apache.org>.
ashb commented on issue #19891:
URL: https://github.com/apache/airflow/issues/19891#issuecomment-1009758226
A separate typestub package is only for when the package itself can't/doesn't want to release typing, so no to that option
--
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