seyoon-lim commented on code in PR #40757:
URL: https://github.com/apache/airflow/pull/40757#discussion_r1747780969


##########
airflow/providers/apache/spark/hooks/spark_submit.py:
##########
@@ -370,10 +448,12 @@ def _build_spark_submit_command(self, application: str) 
-> list[str]:
             connection_cmd += ["--executor-memory", self._executor_memory]
         if self._driver_memory:
             connection_cmd += ["--driver-memory", self._driver_memory]
-        if self._keytab:
-            connection_cmd += ["--keytab", self._keytab]
-        if self._principal:
-            connection_cmd += ["--principal", self._principal]
+        keytab = self._get_keytab()
+        if keytab:
+            connection_cmd += ["--keytab", keytab]
+        principal = self._get_principal()

Review Comment:
   This part was added because the `_resolve_connection` method might throw an 
exception(before 
[here](https://github.com/apache/airflow/pull/40757/files/b27cd4f12a86d2f0bb20720b8fd0c68ed6a5573a#diff-aa05e2f3d2472084951026f8344dbc7134cb1e3f94d626127ab9dee50fbde2c6R294))
 if the principal and keytab of conn_data are assigned as None, as seen 
[here](https://github.com/apache/airflow/pull/40757/files/b27cd4f12a86d2f0bb20720b8fd0c68ed6a5573a#diff-aa05e2f3d2472084951026f8344dbc7134cb1e3f94d626127ab9dee50fbde2c6R261-R262)
   
   ~~However, now that I think about it, setting the default values of 
conn_data to self._keytab and self._principal might resolve this issue as per 
your feedback.
   This seems to be a matter of choice, so if you think it needs to be changed, 
please let me know and I'll make the adjustments!~~



-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to