Use `set` instead of `list` for dag's tags

apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows

https://airflow.apache.org/

Apache License 2.0

36.32k stars 14.09k forks source link

Use `set` instead of `list` for dag's tags #41420

Closed Avihais12344 closed 3 days ago

Avihais12344 commented 1 month ago

Description

Instead of using list for holding the tags of a specific dag (as we can see at the docs), I suggest using set instead, specifically MutableSet for the type hinting, and python set as the default implementation.

Use case/motivation

I would want to add multiple tags (even the same ones) faster, without worrying about duplicates, and reduce the memory usage because of it. Also, it's good to check if a certain tag already exists.

Related issues

No response

Are you willing to submit a PR?

[X] Yes I am willing to submit a PR!

Code of Conduct

[X] I agree to follow this project's Code of Conduct

boring-cyborg[bot] commented 1 month ago

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

sunank200 commented 4 weeks ago

@Avihais12344 Thanks for raising the issue. I had follow-up questions:

Will this change to set break any existing DAGs or workflows expecting a list? I think it would. Wouldn't this be breaking change?
Are there any edge cases or performance regressions to consider with this change?
How would we ensure the use of set is consistent across the codebase, particularly in how tags are stored/retrieved?

Avihais12344 commented 3 weeks ago

Hello @sunank200, thanks for responding.

It would break existing code expecting tags as list, that's why I wanted to discuss it here. The interface is different from list, but other than that, it would not be too much than different, we can think of it as a unique list. So I think there would not be edge case, as we alreay take the unique items from the tags list.

We can ensure the use of set accross the codebase by using typing and linters that would enforce the right types. I would also use the collections.abc type of MutableSet instead of the builtin set, to allow any type of set.

Avihais12344 commented 3 weeks ago

Created a PR for this here.

github-actions[bot] commented 1 week ago

This issue has been automatically marked as stale because it has been open for 14 days with no response from the author. It will be closed in next 7 days if no further activity occurs from the issue author.

github-actions[bot] commented 3 days ago

This issue has been closed because it has not received response from the issue author.