Databricks SQL sensor

[harishkesavarao]

Databricks SQL Sensor.

[harishkesavarao]

@potiuk @hussein-awala @alexott @eladkal @josh-fell @bilalaslamseattle @o-nikolas can you please review my new PR here since #30428 was almost approved but ran into the git UI rebase bug so I closed it and opened a new PR? Thank you?

[harishkesavarao]

@hussein-awala to avoid the UI rebase bug causing a problem, what steps did you execute/run to locally clean your branch when you faced the problem?

And is there a way to reuse the same PR when the problem occurs or do we need to open new PRs for the force pushes?

[potiuk]

@hussein-awala to avoid the UI rebase bug causing a problem, what steps did you execute/run to locally clean your branch when you faced the problem?

And is there a way to reuse the same PR when the problem occurs or do we need to open new PRs for the force pushes?

You need to have your branch rebased onto the apache/main and you need to push your branch with --force to your fork. That should be enough. See the command line instructiin in "how to rebase" section in CONTRIBUTING docs.

[pankajastro]

not quite sure, but I feel the operator is a better choice here not sensor wdyt?

[harishkesavarao]

Thanks for your review @pankajastro. This was reviewed extensively before via #30428 and @josh-fell, @o-nikolas and I agreed that we can keep the hook here because the operator isn't going to do anything more than what the hook does for this sensor.

[pankajastro]

By design, a sensor should only wait for something. My concern here is we are running a query which will take the worker slot for the entire task execution time so there can be a problem when we run it in reschedule mode.

[pankajastro]

Also just curious how this is different than DatabricksSqlOperator

airflow/airflow/providers/databricks/operators/databricks_sql.py

Line 36 in fa4fb1f

class DatabricksSqlOperator(SQLExecuteQueryOperator):

[harishkesavarao]

By design, a sensor should only wait for something. My concern here is we are running a query which will take the worker slot for the entire task execution time so there can be a problem when we run it in reschedule mode.

@pankajastro Since this is an SQL sensor, running a SQL query to check/poke for existence of results is necessary. You are right about the sensor waiting for something in general, but this sensor doesn't wait indefinitely until it receives results, it immediately returns either a True or False based on whether the query returns any results at all. This is useful if you only want to know whether there are results but not want to know what they are, and take action based on it.

[harishkesavarao]

Also just curious how this is different than DatabricksSqlOperator

airflow/airflow/providers/databricks/operators/databricks_sql.py

Line 36 in fa4fb1f

class DatabricksSqlOperator(SQLExecuteQueryOperator):

In the context of Databricks, the SQL operator may not appear functionally different from the sensor in that both execute SQL queries, but the sensor can be used as an upstream dependency to check whether a query result contains data or not and then do something else with a downstream task, while you cannot do this with the operator. The sensor check returns just a True or False and not actual query results, while the operator just executes the query and returns the results back to the logger.

[harishkesavarao]

@josh-fell @o-nikolas @eladkal requesting your reviews for the PR. I made all the changes you suggested over the past few days and we were almost close to merging when reviewing #30428. I had to close it due to the UI rebase bug, so this is just a copy of that PR.

[alexott]

lgtm

[alexott]

@josh-fell @potiuk it looks like all comments were addressed

[o-nikolas]

Thanks for sticking with this one and being gracious in taking all the feedback @harishkesavarao!

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.