[GitHub] [airflow] ashb commented on a change in pull request #16696: Improve graph view refresh

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

ashb commented on a change in pull request #16696:
URL: https://github.com/apache/airflow/pull/16696#discussion_r660520186



##########
File path: airflow/www/static/js/graph.js
##########
@@ -364,14 +364,34 @@ function setFocusMap(state) {
 
 const stateIsSet = () => !!Object.keys(stateFocusMap).find((key) => stateFocusMap[key]);
 
+let prevTis;
+
 function handleRefresh() {
   $('#loading-dots').css('display', 'inline-block');
   $.get(getTaskInstanceURL)
     .done(
       (tis) => {
+        // only refresh if the data has changed
+        if (prevTis !== tis) {
         // eslint-disable-next-line no-global-assign
-        taskInstances = JSON.parse(tis);
-        updateNodesStates(taskInstances);
+          taskInstances = JSON.parse(tis);
+          const states = Object.values(taskInstances).map((ti) => ti.state);
+          updateNodesStates(taskInstances);
+
+          // end refresh if all states are final
+          if (!states.some((state) => (
+            state === null
+            || state === 'running'
+            || state === 'scheduled'
+            || state === 'queued'
+            || state === 'up_for_retry'
+            || state === 'up_for_reschedule'

Review comment:
       We should invert this -- there are fewer terminal states (and less chance of us adding any new ones  - we added a new on in https://github.com/apache/airflow/pull/16681 for instance)
   
   
   So something like
   
   ```suggestion
               ["success", "failed", "upstream_failed", "skipped", "removed"].indexOf(state) !== -1
   ```




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

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