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 2023/01/12 00:59:57 UTC
[GitHub] [airflow] travis-cook-sfdc opened a new issue, #28870: Support multiple buttons from `operator_extra_links`
travis-cook-sfdc opened a new issue, #28870:
URL: https://github.com/apache/airflow/issues/28870
### Description
`BaseOperatorLink` should support returning multiple links:
```
class ButtonLink::
def __init__(self, name: str, link: str) -> None:
self.name = name
self.link = link
class BaseOperatorLink(metadataclass=ABCMeta):
...
@abstractmethod
def get_links(self, operator: BaseOperator, dttm: datetime) -> List[ButtonLink]:
pass
```
### Use case/motivation
My company has implemented a "MultiExternalTaskSensor" that allows a single task to wait for multiple other tasks to complete.
`ExternalTaskSensor` automatically supports `operator_extra_links` to provide a link to the upstream DAG page, but this is impossible to do with `Multi...` since there's an arbitrary number of buttons that need to show up based on the number of tasks that are being waited on.
Another benefit of allowing this would be that the buttons could support dynamic names, which would allow information about the task (like it's state) to present in the button text.
This would add an additional challenge while the button data was loading, since the name might not be known. At this point, there could either be a default name fallback or a simple loading spinner while we wait for all button names to return.
There would need to be changes to the `/extra_links` API and `dag.js`, but it seems feasible.
It could be implement along the lines of:
```python
class ButtonLink:
def __init__(self, name: str, link: str) -> None:
self.name = name
self.link = link
class BaseOperatorLink(metaclass=ABCMeta):
...
@abstractmethod
def get_links(self, operator: BaseOperator, dttm: datetime) -> List[ButtonLink]:
pass
```
### Related issues
_No response_
### Are you willing to submit a PR?
- [ ] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1396486390
Using property is not supported because internally Airflow expects an iterable. Although this gives me an idea, we may be able to detect `operator_extra_links` is a property and 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] travis-cook-sfdc commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
travis-cook-sfdc commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1387745349
> nice discussion. I see the problem now, and yeah, I agree having support for dynamic collection of links in operator is a better idea
@potiuk - It does seem like `operator_extra_links` in theory would support a dynamic collection of links.
https://github.com/apache/airflow/blob/main/airflow/models/baseoperator.py#L634
Assuming you write `operator_extra_links` in your operator class as
```
@property
def operator_extra_links(self, ...):
...
return (x, y, ...)
```
I think the issue has to do with how those extra links are registered as plugins but perhaps I'm not understanding how this could be implemented correctly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1380589626
> How is this different from implementing multiple OperatorLink classes for the operator?
Ah ... I completely forgot that we can do that.... I guess we can close that one then - if this does not work for you @travis-cook-sfdc , I will close it. We can re-open if you think that using multiple links is not good and have some good justification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1379906282
As long as it backwards compatible - sure. If you would like to contribute it, that works ld be great - it does not pass the bar of being super simple but for experienced person it might be interesting one to learn how things work. Marking as good first 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] travis-cook-sfdc commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
travis-cook-sfdc commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1387616659
@Taragolis - Interesting example. I thought I had tried something similar, but I ran into problems when I attempted to register subclass of `BaseOperatorLink` as an Airflow plugin since I thought I needed a known cardinality of links. Is the automatic index the magic that makes this 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 closed issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
potiuk closed issue #28870: Support multiple buttons from `operator_extra_links`
URL: https://github.com/apache/airflow/issues/28870
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] travis-cook-sfdc commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
travis-cook-sfdc commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1387738430
After a little more investigation, I think that example only works because the extra link is registered as a provider which doesn't require explicit instantiation. If you want to register it as a plugin, i.e.:
```
class MultiLinksPlugin(AirflowPlugin):
name = "extra_link_plugin"
operator_extra_links = [
BigQueryConsoleIndexableLink(0),
]
```
You have to specify an index which forces an explicit number of buttons at a time when it's unknown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1379674340
Thanks for opening your first issue here! Be sure to follow the issue template!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #28870: Support multiple buttons from `operator_extra_links`
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1410058128
Assuming we go with the `property` approach, the first blocker would be operator serialisation; currently we simply assume `operator_extra_links` is a class-level list:
https://github.com/apache/airflow/blob/b5b1fae2dfe92751d6aaaace00009ce29625095b/airflow/serialization/serialized_objects.py#L1086-L1109
Here is a report where a property object crashes on serialisation: #25243
Everything else should be easy enough if we can fix 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 issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1386746732
nice discussion. I see the problem now, and yeah, I agree having support for dynamic collection of links in operator is a better 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 commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1386748498
Updated the description to reflect 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] uranusjr commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1379908954
How is this different from implementing multiple OperatorLink classes for the operator?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] travis-cook-sfdc commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
travis-cook-sfdc commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1386213733
This is different than implementing multiple OperatorLinks for a few reasons:
- The number of links required is unknown at instantiation. It's dependent on the instance of the operator and not the generic operator.
- The button name is hardcoded by the link, but in this case, you'd want it to be dynamic based on the task name.
Imagine this operator:
```
class ExternalTask:
dag_id: str
task_id: str
class MultiExternalTaskSensor(BaseSensorOperator):
def __init__(external_tasks: List[ExternalTask], ...):
....
```
And then the following dags:
```
dag_1 = Dag(...)
wf_upstreams = MultiExternalTaskSensor(
task_id="wf_upstreams",
dag=dag_1,
external_tasks=[
ExternalTask(dag_id="upstream_foo", task_id="bar"),
ExternalTask(dag_id="upstream_bat", task_id="baz")
]
)
...
dag_2 = Dag(...)
wf_upstreams = MultiExternalTaskSensor(
task_id="wf_upstreams",
dag=dag_2,
external_tasks=[
ExternalTask(dag_id="upstream_foo1", task_id="bar1"),
ExternalTask(dag_id="upstream_foo2", task_id="bar2"),
ExternalTask(dag_id="upstream_foo3", task_id="bar3"),
]
)
```
When you open the modal for `dag_1.wf_upstreams`, you should 2 buttons with the name `upstream_foo` and `upstream_bar`.
When you open the modal for `dag_2.wf_upstreams`, you should see 3 buttons with the names `upstream_foo1`, `upstream_foo2`, and `upstream_foo3`.
AFAIK, `BaseOperatorLink` doesn't allow you to set a dynamic number of links based on a task instance attribute nor does it allow you to set dynamic button names. I'd be delighted to be wrong 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] Taragolis commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1386245408
https://github.com/apache/airflow/blob/4f91931b359f76ae38272c727bfe21a18a470f2b/airflow/providers/google/cloud/operators/bigquery.py#L85-L107
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #28870: Support multiple buttons from `operator_extra_links`
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1386457850
We can probably add something like
```python
class MyOperator(BaseOperator):
operator_extra_links = MyOperatorExtraLinkCollection() # Not a list of links!
```
to encapsulate this pattern. Probably makes more sense than having a `BaseOperatorLink` return multiple links.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] travis-cook-sfdc commented on issue #28870: Support multiple buttons from `operator_extra_links`
Posted by "travis-cook-sfdc (via GitHub)" <gi...@apache.org>.
travis-cook-sfdc commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1402800228
I'm interesting in potentially implementing this change, but it's not clear to me exactly what the implementation should look like.
Can anyone provide a summary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Support multiple buttons from `operator_extra_links` [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #28870:
URL: https://github.com/apache/airflow/issues/28870#issuecomment-1999849798
Noting that https://github.com/apache/airflow/issues/25243 is resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org