KoviAnusha commented on code in PR #57704:
URL: https://github.com/apache/airflow/pull/57704#discussion_r2485274075


##########
providers/tableau/src/airflow/providers/tableau/operators/tableau.py:
##########
@@ -116,18 +118,25 @@ def execute(self, context: Context) -> str:
 
             resource_id = self._get_resource_id(tableau_hook)
 
-            response = method(resource_id)
-
-            job_id = response.id
-
-            if self.method == "refresh":
-                if self.blocking_refresh:
-                    if not tableau_hook.wait_for_state(
-                        job_id=job_id,
-                        check_interval=self.check_interval,
-                        target_state=TableauJobFinishCode.SUCCESS,
-                    ):
-                        raise TableauJobFailedException(f"The Tableau Refresh 
{self.resource} Job failed!")
+            job_item: JobItem | None
+            if self.resource == "tasks" and self.method == "run":
+                task_item = resource.get_by_id(resource_id)
+                response = method(task_item)
+                job_item = JobItem.from_response(response, 
tableau_hook.server.namespace)[0]
+            else:
+                job_item = method(resource_id)
+
+            if self.method in ("delete", "remove") or job_item is None:
+                return None
+
+            job_id = job_item.id
+
+            if self.blocking_refresh and not tableau_hook.wait_for_state(
+                job_id=job_id,
+                check_interval=self.check_interval,
+                target_state=TableauJobFinishCode.SUCCESS,
+            ):
+                raise TableauJobFailedException(f"The Tableau Refresh 
{self.resource} Job failed!")

Review Comment:
   I believe logging the job ID would make it easier for debugging.



##########
providers/tableau/src/airflow/providers/tableau/operators/tableau.py:
##########
@@ -116,18 +118,25 @@ def execute(self, context: Context) -> str:
 
             resource_id = self._get_resource_id(tableau_hook)
 
-            response = method(resource_id)
-
-            job_id = response.id
-
-            if self.method == "refresh":
-                if self.blocking_refresh:
-                    if not tableau_hook.wait_for_state(
-                        job_id=job_id,
-                        check_interval=self.check_interval,
-                        target_state=TableauJobFinishCode.SUCCESS,
-                    ):
-                        raise TableauJobFailedException(f"The Tableau Refresh 
{self.resource} Job failed!")
+            job_item: JobItem | None
+            if self.resource == "tasks" and self.method == "run":
+                task_item = resource.get_by_id(resource_id)
+                response = method(task_item)
+                job_item = JobItem.from_response(response, 
tableau_hook.server.namespace)[0]

Review Comment:
   from_response(...)[0] assumes a non-empty list. If the API returns an empty 
collection, we would get an IndexError. It would be good to guard with if 
response or raise a clearer AirflowException when empty.



-- 
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]

Reply via email to