boittega commented on a change in pull request #15794:
URL: https://github.com/apache/airflow/pull/15794#discussion_r630920717
##########
File path: docs/apache-airflow-providers-apache-spark/connections/spark.rst
##########
@@ -27,7 +27,7 @@ The Apache Spark connection type enables connection to Apache
Spark.
Default Connection IDs
----------------------
-Spark Submit and Spark JDBC hooks and operators use ``spark_default`` by
default, Spark SQL hooks and operators point to ``spark_sql_default`` by
default, but don't use it.
+Spark Submit and Spark JDBC hooks and operators use ``spark_default`` by
default, Spark SQL hooks and operators point to ``spark_sql_default`` by
default, but its usage is optional.
Review comment:
change to `use spark_sql_default` instead of `point to
spark_sql_default`.
I would completely remove the `, but its usage is optional`, I think it is
the normal behaviour for any connection.
```suggestion
Spark Submit and Spark JDBC hooks and operators use ``spark_default`` by
default, Spark SQL hooks and operators use ``spark_sql_default`` by default.
```
##########
File path: airflow/providers/apache/spark/hooks/spark_sql.py
##########
@@ -72,13 +77,33 @@ def __init__(
executor_memory: Optional[str] = None,
keytab: Optional[str] = None,
principal: Optional[str] = None,
- master: str = 'yarn',
+ master: Optional[str] = None,
name: str = 'default-name',
num_executors: Optional[int] = None,
verbose: bool = True,
- yarn_queue: str = 'default',
+ yarn_queue: Optional[str] = None,
) -> None:
super().__init__()
+
+ try:
+ conn: "Optional[Connection]" = self.get_connection(conn_id)
+ except AirflowNotFoundException:
+ conn = None
+ options = {}
+ else:
+ options = conn.extra_dejson
+
+ # Set arguments to values set in Connection if not explicitly provided.
+ if master is None:
+ if conn is None:
+ master = "yarn"
+ elif conn.port:
+ master = f"{conn.host}:{conn.port}"
+ else:
+ master = conn.host
+ if yarn_queue is None:
+ yarn_queue = options.get("queue", "default")
+
self._sql = sql
self._conf = conf
self._conn = self.get_connection(conn_id)
Review comment:
receive the `conn` variable here.
```suggestion
self._conn = conn
```
##########
File path: airflow/providers/apache/spark/hooks/spark_sql.py
##########
@@ -72,13 +77,33 @@ def __init__(
executor_memory: Optional[str] = None,
keytab: Optional[str] = None,
principal: Optional[str] = None,
- master: str = 'yarn',
+ master: Optional[str] = None,
name: str = 'default-name',
num_executors: Optional[int] = None,
verbose: bool = True,
- yarn_queue: str = 'default',
+ yarn_queue: Optional[str] = None,
) -> None:
super().__init__()
+
+ try:
+ conn: "Optional[Connection]" = self.get_connection(conn_id)
+ except AirflowNotFoundException:
+ conn = None
+ options = {}
+ else:
+ options = conn.extra_dejson
+
+ # Set arguments to values set in Connection if not explicitly provided.
+ if master is None:
+ if conn is None:
+ master = "yarn"
+ elif conn.port:
+ master = f"{conn.host}:{conn.port}"
+ else:
+ master = conn.host
+ if yarn_queue is None:
+ yarn_queue = options.get("queue", "default")
Review comment:
On the `self._yarn_queue = yarn_queue` bellow you can add the above
logic like this:
```
self._yarn_queue = yarn_queue or options.get("queue", "default")
````
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
