[GitHub] [airflow] bbenzikry commented on a change in pull request #10023: spark-on-k8s sensor - add driver logs

[GitBox <gi...@apache.org>]

bbenzikry commented on a change in pull request #10023:
URL: https://github.com/apache/airflow/pull/10023#discussion_r471736799



##########
File path: airflow/providers/cncf/kubernetes/hooks/kubernetes.py
##########
@@ -99,19 +97,76 @@ def create_custom_resource_definition(self,
                 version=version,
                 namespace=namespace,
                 plural=plural,
-                body=body
+                body=body,
             )
             self.log.debug("Response: %s", response)
             return response
         except client.rest.ApiException as e:
-            raise AirflowException("Exception when calling -> create_custom_resource_definition: %s\n" % e)
-
-    def get_custom_resource_definition(self,
-                                       group: str,
-                                       version: str,
-                                       plural: str,
-                                       name: str,
-                                       namespace: Optional[str] = None):
+            raise AirflowException(
+                "Exception when calling -> create_custom_resource_definition: %s\n" % e
+            )
+
+    def get_pod_log_stream(
+        self,
+        pod_name: str,
+        container: Optional[str] = "",
+        namespace: Optional[str] = None,
+    ) -> Tuple[watch.Watch, Generator[str, None, None]]:
+        """
+        Retrieves a log stream for a container in a kubernetes pod.
+
+        :param pod_name: pod name
+        :type pod_name: str
+        :param container: container name
+        :type version: str
+        :param namespace: kubernetes namespace
+        :type namespace: str
+        """
+
+        api = client.CoreV1Api(self.get_conn())
+        watcher = watch.Watch()
+        return (
+            watcher,
+            watcher.stream(

Review comment:
       This assumes a role similar to the one below ( specific namespace role or ClusterRole. In any case the operator is constrained to the job namespace ) 
   
   ```yaml
   - apiGroups: [""]
     resources: ["pods", "pods/log"]
     verbs: ["get", "watch", "list"]
   ```
   It's important to mention that I opted against using the watcher, but left this here for future use ( I'm using ``read_namespaced_pod_log``, which still requires the mentioned permissions )
   
   Should we add a manifest that deals with it for a system test? similar to RBAC yamls from the operator here https://github.com/apache/airflow/blob/7d24b088cd736cfa18f9214e4c9d6ce2d5865f3d/tests/providers/cncf/kubernetes/operators/test_spark_kubernetes_system.py#L36
   
   In any case I don't think it should block, as it depends on explicitly adding the flag




----------------------------------------------------------------
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:
users@infra.apache.org