pierrejeambrun commented on code in PR #46326:
URL: https://github.com/apache/airflow/pull/46326#discussion_r1937641504


##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, 
isPaused, skipConfirm }: Pr
   const { onClose, onOpen, open } = useDisclosure();
 
   const onSuccess = async () => {
-    await queryClient.invalidateQueries({
-      queryKey: [useDagServiceGetDagsKey],
-    });
+    // We can just update dag details
+    
queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ 
dagId }), (oldData) =>
+      oldData
+        ? {
+            ...oldData,
+            is_paused: !oldData.is_paused,
+          }
+        : undefined,
+    );

Review Comment:
   I'm not super fan of this.
   
   If we add some business logic and side effect on pausing / unpausing a dag 
in the backend,  this will start failing.
   
   We are unnecessary coupling the back and the front on that part. I think an 
extra GET is really not that expensive and worth the trouble. (if we generalize 
that pattern we might end up with issues)



##########
airflow/ui/src/components/TogglePause.tsx:
##########
@@ -42,12 +43,19 @@ export const TogglePause = ({ dagDisplayName, dagId, 
isPaused, skipConfirm }: Pr
   const { onClose, onOpen, open } = useDisclosure();
 
   const onSuccess = async () => {
-    await queryClient.invalidateQueries({
-      queryKey: [useDagServiceGetDagsKey],
-    });
+    // We can just update dag details
+    
queryClient.setQueryData<DAGDetailsResponse>(UseDagServiceGetDagDetailsKeyFn({ 
dagId }), (oldData) =>
+      oldData
+        ? {
+            ...oldData,
+            is_paused: !oldData.is_paused,
+          }
+        : undefined,
+    );

Review Comment:
   I'm not super fan of this.
   
   If we add some business logic and side effect on pausing / unpausing a dag 
in the backend,  this will start failing. (data will be de-synced)
   
   We are unnecessary coupling the back and the front on that part. I think an 
extra GET is really not that expensive and worth the trouble. (if we generalize 
that pattern we might end up with issues)



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