SameerMesiah97 commented on code in PR #68277:
URL: https://github.com/apache/airflow/pull/68277#discussion_r3384840071
##########
providers/apache/spark/src/airflow/providers/apache/spark/operators/spark_submit.py:
##########
@@ -134,6 +134,12 @@ class SparkSubmitOperator(ResumableJobMixin, BaseOperator):
omitted, Kerberos-enabled Spark connections with both ``keytab`` and
``principal`` configured use ``requests-kerberos`` automatically.
Defaults to ``None`` (no auth for non-Kerberos connections).
+ :param deferrable: If ``True``, submits the job then defers to
+ ``SparkDriverTrigger``; the worker slot is freed while the trigger
+ polls the Spark REST API. On crash the trigger is re-created from
+ its serialised state (no reconnect needed). On user-clear, execute()
+ runs again and a fresh job is submitted.
+ If ``False`` (default), the sync ``ResumableJobMixin`` path is used.
Review Comment:
This docstring entry is a bit too verbose. I don't think there is a need to
go deep into the mechanies of deferrable mode here. I understand you want to
express an adjacent development i.e. `ResumableJobMixin` but I believe that
might be better suited for a comment. I think you should use this instead:
`:param deferrable: Run operator in deferrable mode.`
##########
providers/apache/spark/src/airflow/providers/apache/spark/operators/spark_submit.py:
##########
@@ -134,6 +134,12 @@ class SparkSubmitOperator(ResumableJobMixin, BaseOperator):
omitted, Kerberos-enabled Spark connections with both ``keytab`` and
``principal`` configured use ``requests-kerberos`` automatically.
Defaults to ``None`` (no auth for non-Kerberos connections).
+ :param deferrable: If ``True``, submits the job then defers to
+ ``SparkDriverTrigger``; the worker slot is freed while the trigger
+ polls the Spark REST API. On crash the trigger is re-created from
+ its serialised state (no reconnect needed). On user-clear, execute()
+ runs again and a fresh job is submitted.
+ If ``False`` (default), the sync ``ResumableJobMixin`` path is used.
Review Comment:
This docstring entry is a bit too verbose. I don't think there is a need to
go deep into the mechanies of deferrable mode here. I understand you want to
express an adjacent development i.e. `ResumableJobMixin` but I believe that
might be better suited for a comment. I think you should use this instead:
`:param deferrable: Run operator in deferrable mode.`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
