You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/26 23:13:16 UTC
[GitHub] [airflow] jedcunningham opened a new pull request, #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
jedcunningham opened a new pull request, #25323:
URL: https://github.com/apache/airflow/pull/25323
Closes: #25241
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 a diff in pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25323:
URL: https://github.com/apache/airflow/pull/25323#discussion_r932030957
##########
airflow/www/views.py:
##########
@@ -353,6 +354,8 @@ def set_overall_state(record):
'label': item.label,
'extra_links': item.extra_links,
'is_mapped': item.is_mapped,
+ 'has_outlet_datasets': any(isinstance(i, Dataset) for i in getattr(item, "_outlets", [])),
Review Comment:
I suspect they are disconnected for no good reason (or no reason that exists anymore) and `_outlets` should just be removed (which we can do, as it's a private attribute after all)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi merged pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #25323:
URL: https://github.com/apache/airflow/pull/25323
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 a diff in pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25323:
URL: https://github.com/apache/airflow/pull/25323#discussion_r930582441
##########
airflow/www/views.py:
##########
@@ -353,6 +354,8 @@ def set_overall_state(record):
'label': item.label,
'extra_links': item.extra_links,
'is_mapped': item.is_mapped,
+ 'has_outlet_datasets': any(isinstance(i, Dataset) for i in getattr(item, "_outlets", [])),
Review Comment:
I wanted to mention this does not work with mapped operators (it does not have `_outlets`, only `outlets`), but then realised I don’t understand the difference between the two. Is the non-underscore-prefixed one populated by the other? Where?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25323:
URL: https://github.com/apache/airflow/pull/25323#discussion_r931678578
##########
airflow/www/views.py:
##########
@@ -353,6 +354,8 @@ def set_overall_state(record):
'label': item.label,
'extra_links': item.extra_links,
'is_mapped': item.is_mapped,
+ 'has_outlet_datasets': any(isinstance(i, Dataset) for i in getattr(item, "_outlets", [])),
Review Comment:
So I just played with this a bit, and while you can set an outlet in partial, it doesn't actually function. (because it gets serialized into `outlets`?). We might need to investigate that sooner than later, but I'm not sure we should block this PR in the meantime so the UI effort can continue.
The getattr was because mapped operators don't have `_outlets`, so while this doesn't "work", this also doesn't "break" things further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #25323:
URL: https://github.com/apache/airflow/pull/25323#discussion_r931221401
##########
airflow/www/views.py:
##########
@@ -353,6 +354,8 @@ def set_overall_state(record):
'label': item.label,
'extra_links': item.extra_links,
'is_mapped': item.is_mapped,
+ 'has_outlet_datasets': any(isinstance(i, Dataset) for i in getattr(item, "_outlets", [])),
Review Comment:
this is a known issue, [documented here](https://github.com/apache/airflow/projects/16#card-84459116), for someone to look into at some point
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 a diff in pull request #25323: Add `operator` and `has_outlet_datasets` to `/grid_data`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25323:
URL: https://github.com/apache/airflow/pull/25323#discussion_r931731656
##########
airflow/www/views.py:
##########
@@ -353,6 +354,8 @@ def set_overall_state(record):
'label': item.label,
'extra_links': item.extra_links,
'is_mapped': item.is_mapped,
+ 'has_outlet_datasets': any(isinstance(i, Dataset) for i in getattr(item, "_outlets", [])),
Review Comment:
Yeah I think when I implemented MappedOperator I just assumed `outlets` and `_outlets` (also inlets) are sync-ed and didn’t look deep into it. Now I did I don’t understand why there are two attributes to begin with and have no idea how things _should_ 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