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/04/13 20:15:59 UTC
[GitHub] [airflow] alexandrecaze opened a new pull request #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
alexandrecaze opened a new pull request #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277
Add two shapes to the elastic DAG :
- BINARY_TREE
```
t0 -> t1 -> t3 -> t7
| \
| -> t4 -> t8
|
-> t2 -> t5 -> t9
\
-> t6
```
- STAR
```
t0 -> t1
| -> t2
| -> t3
| -> t4
| -> t5
```
and add corresponding tests in `test_scheduler_job`.
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [?] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on issue #8277: [AIRFLOW-8187]
Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on issue #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-614567431
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze edited a comment on issue #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze edited a comment on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613077136
@mik-laj This is a first attempt to tackle https://github.com/apache/airflow/issues/8187
I did not add the GRID shape yet (did not find an elegant way of doing it), but would appreciate your feedback on the way I implemented the STAR and BINARY_TREE.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze edited a comment on issue #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze edited a comment on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613077136
@mik-laj This is a first attempt to tackle https://github.com/apache/airflow/issues/8187
I did not add the GRID shape yet (did not find an elegant way of doing it), but would appreciate your feedback on the way I implemented the star and grid.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409359371
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,79 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[0])
Review comment:
oooh nice
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #8277:
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409300985
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -123,5 +188,11 @@ class DagShape(Enum):
pass
elif SHAPE == DagShape.LINEAR:
chain(*tasks)
+ elif SHAPE == DagShape.BINARY_TREE:
+ chain_as_binary_tree(*tasks)
+ elif SHAPE == DagShape.STAR:
+ chain_as_star(*tasks)
+ elif SHAPE == DagShape.GRID:
+ chain_as_grid(*tasks)
Review comment:
This list will probably grow over time and pylint will start to moan about to many elif cases. So I would suggest to use dictionary:
```python
shape_function_map = {
DagShape.LINEAR: chain,
DagShape.BINARY_TREE: chain_as_binary_tree,
...
}
builder = shape_function_map[SHAPE]
builder(*tasks)
```
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409379193
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -123,5 +188,11 @@ class DagShape(Enum):
pass
elif SHAPE == DagShape.LINEAR:
chain(*tasks)
+ elif SHAPE == DagShape.BINARY_TREE:
+ chain_as_binary_tree(*tasks)
+ elif SHAPE == DagShape.STAR:
+ chain_as_star(*tasks)
+ elif SHAPE == DagShape.GRID:
+ chain_as_grid(*tasks)
Review comment:
fixed ๐
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #8277: WIP : [AIRFLOW-8187]
Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613108966
# [Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=h1) Report
> Merging [#8277](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/44e55049fb76d00bc419f03cc049c819d0006bc1&el=desc) will **decrease** coverage by `0.74%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8277 +/- ##
==========================================
- Coverage 88.41% 87.67% -0.75%
==========================================
Files 937 937
Lines 45230 45240 +10
==========================================
- Hits 39992 39666 -326
- Misses 5238 5574 +336
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=tree) | Coverage ฮ | |
|---|---|---|
| [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `95.74% <100.00%> (+0.05%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `ฮ = absolute <relative> (impact)`, `รธ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=footer). Last update [44e5504...aa95ede](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #8277:
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409415753
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,78 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ tasks[0].set_upstream(tasks[1:])
Review comment:
```suggestion
tasks[0].set_upstream(list(tasks[1:]))
```
This should please mypy. Or we can change sigature of `set_upstream` to accept any of (list, tuple, set).
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on a change in pull request #8277: WIP
: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r408448901
##########
File path: airflow/models/baseoperator.py
##########
@@ -1239,6 +1239,36 @@ def chain(*tasks: Union[BaseOperator, List[BaseOperator]]):
up_t.set_downstream(down_t)
+def chain_as_binary_tree(*tasks: BaseOperator):
Review comment:
Can you move it to elastic_dag.py? We don't want other users to use this method because it is only useful for testing.
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] boring-cyborg[bot] commented on issue #8277:
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-614681915
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613108966
# [Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=h1) Report
> Merging [#8277](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/44e55049fb76d00bc419f03cc049c819d0006bc1&el=desc) will **decrease** coverage by `0.74%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8277 +/- ##
==========================================
- Coverage 88.41% 87.67% -0.75%
==========================================
Files 937 937
Lines 45230 45240 +10
==========================================
- Hits 39992 39666 -326
- Misses 5238 5574 +336
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=tree) | Coverage ฮ | |
|---|---|---|
| [airflow/models/baseoperator.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZW9wZXJhdG9yLnB5) | `95.74% <100.00%> (+0.05%)` | :arrow_up: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| [airflow/providers/mysql/operators/mysql.py](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbXlzcWwvb3BlcmF0b3JzL215c3FsLnB5) | `55.00% <0.00%> (-45.00%)` | :arrow_down: |
| ... and [19 more](https://codecov.io/gh/apache/airflow/pull/8277/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `ฮ = absolute <relative> (impact)`, `รธ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=footer). Last update [44e5504...aa95ede](https://codecov.io/gh/apache/airflow/pull/8277?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze edited a comment on issue #8277:
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze edited a comment on issue #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-614257628
I need some help with the CI, I don't understand why it is failing.
https://apache-airflow.slack.com/archives/CSS36QQS1/p1586977352085500
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409417749
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,78 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ tasks[0].set_upstream(tasks[1:])
Review comment:
just pushed your suggestion thanks ๐
I do not have the experience to judge whether changing the signature of `set_upstream` is appropriate so I'll let you judge of 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
With regards,
Apache Git Services
[GitHub] [airflow] turbaszek commented on a change in pull request #8277:
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
turbaszek commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409299676
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,79 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[0])
Review comment:
```suggestion
tasks[0].set_upstream(tasks[1:])
```
We don't need loop as both set_upstream and set_downstream accept:
`task_or_task_list: Union['BaseOperator', List['BaseOperator']]`
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r408639757
##########
File path: airflow/models/baseoperator.py
##########
@@ -1239,6 +1239,36 @@ def chain(*tasks: Union[BaseOperator, List[BaseOperator]]):
up_t.set_downstream(down_t)
+def chain_as_binary_tree(*tasks: BaseOperator):
Review comment:
fixed ๐
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on issue #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613077136
@mik-laj This is a first attempt to tackle https://github.com/apache/airflow/issues/8187
I did not add the grid yet (did not find an elegant way of doing it), but would appreciate your feedback on the way I implemented the star and grid.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409417749
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,78 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ tasks[0].set_upstream(tasks[1:])
Review comment:
just pushed your suggestion ๐
I do not have the experience to judge whether changing the signature of `set_upstream` is appropriate so I'll let you judge of 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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #8277: [AIRFLOW-8187] Extend
elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] boring-cyborg[bot] commented on issue #8277: WIP :
[AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613076568
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.
- 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://apache-airflow-slack.herokuapp.com/
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on a change in pull request
#8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on a change in pull request #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#discussion_r409379259
##########
File path: scripts/perf/dags/elastic_dag.py
##########
@@ -74,14 +74,79 @@ def safe_dag_id(s: str) -> str:
return re.sub('[^0-9a-zA-Z_]+', '_', s)
-# TODO: We should add more shape types e.g. binary tree
+def chain_as_binary_tree(*tasks: BashOperator):
+ r'''
+ Chain tasks as a binary tree where task i is child of task (i - 1) // 2 :
+
+ t0 -> t1 -> t3 -> t7
+ | \
+ | -> t4 -> t8
+ |
+ -> t2 -> t5 -> t9
+ \
+ -> t6
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[(i - 1) // 2])
+
+
+def chain_as_grid(*tasks: BashOperator):
+ '''
+ Chain tasks as a grid:
+
+ t0 -> t1 -> t2 -> t3
+ | | |
+ v v v
+ t4 -> t5 -> t6
+ | |
+ v v
+ t7 -> t8
+ |
+ v
+ t9
+ '''
+ if len(tasks) > 100 * 99 / 2:
+ raise ValueError('Cannot generate grid DAGs with lateral size larger than 100 tasks.')
+ grid_size = min([n for n in range(100) if n * (n + 1) / 2 >= len(tasks)])
+
+ def index(i, j):
+ '''
+ Return the index of node (i, j) on the grid.
+ '''
+ return int(grid_size * i - i * (i - 1) / 2 + j)
+
+ for i in range(grid_size - 1):
+ for j in range(grid_size - i - 1):
+ if index(i + 1, j) < len(tasks):
+ tasks[index(i + 1, j)].set_downstream(tasks[index(i, j)])
+ if index(i, j + 1) < len(tasks):
+ tasks[index(i, j + 1)].set_downstream(tasks[index(i, j)])
+
+
+def chain_as_star(*tasks: BashOperator):
+ '''
+ Chain tasks as a star (all tasks are children of task 0)
+
+ t0 -> t1
+ | -> t2
+ | -> t3
+ | -> t4
+ | -> t5
+ '''
+ for i in range(1, len(tasks)):
+ tasks[i].set_downstream(tasks[0])
Review comment:
fixed ๐
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #8277: WIP : [AIRFLOW-8187]
Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613693089
DAGs looks 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] alexandrecaze commented on issue #8277: [AIRFLOW-8187]
Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
alexandrecaze commented on issue #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-614257628
I need some help with the CI, I don't understand why it is failing.
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #8277: WIP : [AIRFLOW-8187]
Extend elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #8277: WIP : [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-613693536
Thank you very much for working on this change. I need this very much in my upcoming works.
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #8277: [AIRFLOW-8187] Extend
elastic DAG with a binary tree, grid, star
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #8277: [AIRFLOW-8187] Extend elastic DAG with a binary tree, grid, star
URL: https://github.com/apache/airflow/pull/8277#issuecomment-614260802
Please rebase to latest master. We have done some changes and split the CI jobs Travis <> Github Actions so that we can get better stability.. (see the devlist announcements),
----------------------------------------------------------------
This is an automated message from the 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
With regards,
Apache Git Services