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/12/20 14:17:10 UTC
[GitHub] [airflow] josh-fell opened a new pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
josh-fell opened a new pull request #20423:
URL: https://github.com/apache/airflow/pull/20423
Related: #19891
In a previous PR to fix Mypy errors, the `region` arg was removed from `default_args` in the Alibaba example DAGs. While this did alleviate Mypy errors it removed a valid DAG-authoring feature that was being showcased. This PR returns the `region` arg to `default_args` as well as handling Mypy errors.
> Note: Generally it seems like the first approach to handle these call-arg errors from Mypy related to `default_args` functionality is to add the ignore comments where appropriate. A proper fix for these scenarios seems very non-trivial and this is a good interim solution.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998040530
Should we make region Optional and raise Runtime Error instead as I did in 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] potiuk commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998138088
> Hmm. It seems that the Mypy checks in CI and running `mypy --namespace-packages airflow/providers/alibaba/cloud` locally in a Breeze container have different results. Still showing a call-arg error for `region` even with the stubs.
One more comment - what worked for me, I set the fields to be "Optional[str] = None" in stubs. That was what worked perfectly for 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] github-actions[bot] commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998230961
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#discussion_r772554023
##########
File path: airflow/providers/alibaba/cloud/example_dags/example_oss_bucket.pyi
##########
@@ -0,0 +1,19 @@
+from typing import Optional
+
+class OSSCreateBucketOperator:
+ def __init__(
+ self,
+ region: str = ...,
Review comment:
Ah noe
```suggestion
region: Optional[str] = None
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #20423:
URL: https://github.com/apache/airflow/pull/20423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
josh-fell edited a comment on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998133314
Hmm. It seems that the Mypy checks in CI and running `mypy --namespace-packages airflow/providers/alibaba/cloud` locally in a Breeze container have different results. Still showing a call-arg error for `region` even with the stubs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998041738
See: https://github.com/apache/airflow/pull/20422/files#diff-aec3e30af161525a7290fd9f420169f8a107c929e9b33b613bef4c03dd228c2dR70
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 a change in pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
josh-fell commented on a change in pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#discussion_r772555615
##########
File path: airflow/providers/alibaba/cloud/example_dags/example_oss_bucket.pyi
##########
@@ -0,0 +1,19 @@
+from typing import Optional
+
+class OSSCreateBucketOperator:
+ def __init__(
+ self,
+ region: str = ...,
Review comment:
Oh OK. Let me try this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#discussion_r772555124
##########
File path: airflow/providers/alibaba/cloud/example_dags/example_oss_bucket.pyi
##########
@@ -0,0 +1,19 @@
+from typing import Optional
+
+class OSSCreateBucketOperator:
+ def __init__(
+ self,
+ region: str = ...,
Review comment:
And others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
josh-fell commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-1001266059
There was a typo in an import within the stub file which was causing the stub to not be recognized. @potiuk can you review again when you get a chance please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998085292
Ah nice. Using stub files seems to be a better solution. I will use that in #20422 too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
josh-fell commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998133314
Hmm. It seems that the Mypy checks in CI and running `mypy` locally in Breeze have different results. Still showing a call-arg error for `region` even with the stubs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20423: Reinstate `region` to `default_args` for Alibaba example DAGs
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20423:
URL: https://github.com/apache/airflow/pull/20423#issuecomment-998136651
> Hmm. It seems that the Mypy checks in CI and running `mypy` locally in Breeze have different results. Still showing a call-arg error for `region` even with the stubs.
Not for me #20422 It worked just fine in `Breeze` - same way as in CI.
It could be about .pyc files probably ? (wild guess) Did you actually use breeze or `mypy` in local virtualenv? And maybe you had some locally generated .pyc files ? Because in Breeze. `PYTHONDONTWRITEBYTECODE=true` is set, so .pyc files are not generated, but if you run it locally they coudl be
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org