In DAG dependency detector, use class type instead of class name, related #19582

[davidavdav]

This small PR uses the type of a BaseOperator task, rather than its property task_type, to detect DAG dependence.

This helps in situations like described in #19582, where an operator is derived from ExternalTaskSensor. Using the task_type prevents such a sensor from being detected as a DAG dependence. This PR will allow automatic detection of derived classes.

[boring-cyborg]

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/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (flake8, mypy and type annotations). Our pre-commits 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 Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment 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 for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  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

[kaxil]

Since this is done during serialization time this should be ok, however can you please add test(s)

[davidavdav]

Sure I'll look into how tests work in airflow, and add one, I suppose, for testing that both TriggerDagRunOperator and inherited classes give the expected result.

[dstandish]

@kaxil do you think that perhaps there should be an attribute on task sensor (like a class attribute that is set on it, and which could also be set in other implementations) that we look at (instead of the class itself) so that there could be other implementations of task-sensor-like operators which do not inherit from external task sensor but also have dag dependencies?

so that it needn't necessarily be a subclass. maybe it's a bit too magical, and just thinking aloud, but perhaps we could look at the arguments of the operator to infer whether DagDependency should be created?

[kaxil]

Looks good, can you fix Static Checks please https://github.com/apache/airflow/runs/4281700481?check_suite_focus=true and rebase on latest main branch please.

[davidavdav]

Is that purely formatting / style? I should be able to fix that (but so should the automatic checking I suppose?).

[dstandish]

i think this looks ok but just have some naming nits

[dstandish]

Suggested change

	for Trigger in [TriggerDagRunOperator, DerivedTrigger]:
	for class_ in [TriggerDagRunOperator, DerivedTrigger]:

[dstandish]

@kaxil do you think that perhaps there should be an attribute on task sensor (like a class attribute that is set on it, and which could also be set in other implementations) that we look at (instead of the class itself) so that there could be other implementations of task-sensor-like operators which do not inherit from external task sensor but also have dag dependencies?

so that it needn't necessarily be a subclass. maybe it's a bit too magical, and just thinking aloud, but perhaps we could look at the arguments of the operator to infer whether DagDependency should be created?

[davidavdav]

bump

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.