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 2020/10/05 11:34:51 UTC
[GitHub] [airflow] kishvanchee opened a new pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
kishvanchee opened a new pull request #11277:
URL: https://github.com/apache/airflow/pull/11277
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
This change is related to #11178
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/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/master/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-721446368
Can you please rebase your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
It will help if your squash your commits into single commit first so that there are less conflicts.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703574588
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703574706
The CI and PROD Docker Images for the build are prepared in a separate "Build Image" workflow,
that you will not see in the list of checks (you will see "Wait for images" jobs instead).
You can checks the status of those images in [The workflow run](https://github.com/apache/airflow/actions/runs/289295789)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703744469
[The Build Workflow run](https://github.com/apache/airflow/actions/runs/289727700) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703792547
[The Build Workflow run](https://github.com/apache/airflow/actions/runs/289866879) is cancelling this PR. The job has been cancelled by another workflow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on a change in pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on a change in pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#discussion_r499690924
##########
File path: airflow/operators/dagrun_operator.py
##########
@@ -15,88 +15,14 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+"""This module is deprecated. Please use `airflow.operators.dagrun`."""
-import datetime
-from typing import Dict, Optional, Union
-from urllib.parse import quote
+import warnings
-from airflow.api.common.experimental.trigger_dag import trigger_dag
-from airflow.models import BaseOperator, BaseOperatorLink, DagRun
-from airflow.utils import timezone
-from airflow.utils.decorators import apply_defaults
-from airflow.utils.types import DagRunType
+# pylint: disable=unused-import
+from airflow.operators.dagrun import TriggerDagRunOperator # noqa
-
-class TriggerDagRunLink(BaseOperatorLink):
Review comment:
Got it. Just noticed. I guess I didn't read the conflict properly although when I was working on it the particular `class TriggerDagRunLink(BaseOperatorLink)` didn't exist in the file. I'll fix this now.
Please correct me if I'm wrong, so to fix this, I should finish working on the file, fetch/rebase, then push again. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-732343398
@turbaszek @kaxil Hi, I apologize for the delay. I'll update this by tomorrow afternoon IST.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-704197782
I think I should be updated `operators-and-hooks-ref.rst` too. I'll await further comments for now before doing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #11277: Move dagrun_operator.py to trigger_dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-741259640
Closing in favour of https://github.com/apache/airflow/pull/12933
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703580721
@mik-laj Please review and let me know if I have to change anything.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-731855172
@kishvanchee any update on this one?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703728895
The CI and PROD Docker Images for the build are prepared in a separate "Build Image" workflow,
that you will not see in the list of checks (you will see "Wait for images" jobs instead).
You can checks the status of those images in [The workflow run](https://github.com/apache/airflow/actions/runs/289732173)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703783997
I just noticed. In tests it is named as `test_trigger_dagrun_operator_conf` and `test_trigger_dagrun_operator_templated_conf`. Should those be changed to `test_trigger_dagrun_conf` and `test_trigger_dagrun_templated_conf`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to trigger_dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-735226088
The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703783997
I just noticed. In tests it is named as `test_trigger_dagrun_operator_conf` and `test_trigger_dagrun_operator_templated_conf`. Should those be changed to `test_trigger_dagrun_conf` and `test_trigger_dagrun_templated_conf`
@turbaszek please review the changes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-705070368
> I started `find`ing `.rst` only recently now.
And we need to update those files too. We should use new path not only in code but also in documentation.
I'm personally using PyCharm which has a refactor option that automatically searches for related occurrences and fixes them. This helps a lot with automating. Eventually one can use `cmd + r` to find and replace a fragment of text - this would do the trick in this case as we are only changing import path
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#discussion_r499652872
##########
File path: airflow/operators/dagrun_operator.py
##########
@@ -15,88 +15,14 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+"""This module is deprecated. Please use `airflow.operators.dagrun`."""
-import datetime
-from typing import Dict, Optional, Union
-from urllib.parse import quote
+import warnings
-from airflow.api.common.experimental.trigger_dag import trigger_dag
-from airflow.models import BaseOperator, BaseOperatorLink, DagRun
-from airflow.utils import timezone
-from airflow.utils.decorators import apply_defaults
-from airflow.utils.types import DagRunType
+# pylint: disable=unused-import
+from airflow.operators.dagrun import TriggerDagRunOperator # noqa
-
-class TriggerDagRunLink(BaseOperatorLink):
Review comment:
You missed moving this class as well as operator_extra_links in the `TriggerDagRunOperator`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-732966027
@kaxil @turbaszek Can you please have a look? I have rebased and pushed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703783997
I just noticed. In tests it is named as `test_trigger_dagrun_operator_conf` and `test_trigger_dagrun_operator_templated_conf`. Should those be changed to `test_trigger_dagrun_conf` and `test_trigger_dagrun_templated_conf` ?
@turbaszek please review the changes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-732966027
@kaxil @turbaszek Can you please have a look? I have rebased and 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee edited a comment on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee edited a comment on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-704197782
I think I should be updating `operators-and-hooks-ref.rst` too. I'll await further comments for now before doing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
turbaszek commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-704281395
> Wouldn't trigger_dagrun be better?
+1 for this name and I second Kamil that there's no need to change names of tests but yeah, we need update in `operators-and-hooks-ref.rst` and everywhere in documentation (I would use IDE to look for this operator in the whole Airflow code base)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on a change in pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on a change in pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#discussion_r499710423
##########
File path: airflow/operators/dagrun_operator.py
##########
@@ -15,88 +15,14 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+"""This module is deprecated. Please use `airflow.operators.dagrun`."""
-import datetime
-from typing import Dict, Optional, Union
-from urllib.parse import quote
+import warnings
-from airflow.api.common.experimental.trigger_dag import trigger_dag
-from airflow.models import BaseOperator, BaseOperatorLink, DagRun
-from airflow.utils import timezone
-from airflow.utils.decorators import apply_defaults
-from airflow.utils.types import DagRunType
+# pylint: disable=unused-import
+from airflow.operators.dagrun import TriggerDagRunOperator # noqa
-
-class TriggerDagRunLink(BaseOperatorLink):
Review comment:
I have pushed with the corrections. @turbaszek please have a look.
Apologies for the previous commit. I messed up with the rebase.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-703579980
[The Build Workflow run](https://github.com/apache/airflow/actions/runs/289295789) is cancelling this PR. The job has been cancelled by another workflow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-704157344
> I just noticed. In tests it is named as test_trigger_dagrun_operator_conf and test_trigger_dagrun_operator_templated_conf. Should those be changed to test_trigger_dagrun_conf and test_trigger_dagrun_templated_conf ?
This is not necessary in my opinion.
> Move dagrun_operator.py to dagrun.py
I have doubts if this is a good name for a module. Wouldn't `trigger_dagrun` be better? @turbaszek WDYT
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kishvanchee commented on pull request #11277: Move dagrun_operator.py to dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
kishvanchee commented on pull request #11277:
URL: https://github.com/apache/airflow/pull/11277#issuecomment-704289191
@turbaszek Thanks I'll make the change.
Maybe this isn't the right place to ask, if so please let me know. Currently my workflow is as follows:
```
$ find . \( -name '*.py' -or -name '*.rst' \) -type f -print0 | xargs -0 grep --color -n 'airflow.operators.<op>'
# grep results which I view before making a change
$ find . \( -name '*.py' -or -name '*.rst' \) -type f -print0 | xargs -0 sed -i 's/airflow.operators.<op>/airflow.operators.<OP>/g'
```
I started `find`ing `.rst` only recently now. Since you suggest an IDE, can you please recommend a better workflow maybe for working with airflow codebase?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb closed pull request #11277: Move dagrun_operator.py to trigger_dagrun.py (#11178)
Posted by GitBox <gi...@apache.org>.
ashb closed pull request #11277:
URL: https://github.com/apache/airflow/pull/11277
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org