Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workflows error state #1556

Closed

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Nov 21, 2023

Built on #1549

Remove the "error" option from the workflow state filter.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders oliver-sanders added this to the 2.3.0 milestone Nov 21, 2023
@oliver-sanders oliver-sanders self-assigned this Nov 21, 2023
* "error" is not a workflow state, it shouldn't be user-facing.
@oliver-sanders
Copy link
Member Author

(rebased)

@MetRonnie
Copy link
Member

Is there no such thing as the error state, then? If so, why not remove here

export class WorkflowState extends Enumify {
static RUNNING = new WorkflowState('running', mdiPlayCircle)
static PAUSED = new WorkflowState('paused', mdiPauseCircle)
static STOPPING = new WorkflowState('stopping', mdiSkipNextCircle)
static STOPPED = new WorkflowState('stopped', mdiStopCircle)
static ERROR = new WorkflowState('error', mdiHelpCircle)

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Nov 27, 2023

I would, but unfortunately, workflows sometimes end up in this state. I think there's some logic that makes this the default if state is null?

@MetRonnie
Copy link
Member

Ah, there's no ERROR state here: https://github.com/cylc/cylc-flow/blob/642ab8d22652f0aa3d5576d29736355a98b337d0/cylc/flow/workflow_status.py#L44-L58

class WorkflowStatus(Enum):
    """The possible statuses of a workflow."""

    PAUSED = "paused"
    """Workflow will not submit any new jobs."""

    RUNNING = "running"
    """Workflow is running as normal."""

    STOPPING = "stopping"
    """Workflow is in the process of shutting down."""

    STOPPED = "stopped"
    """Workflow is not running."""

What I would do is this:

diff --git a/src/model/WorkflowState.model.js b/src/model/WorkflowState.model.js
index 2562d8f9..8fed318b 100644
--- a/src/model/WorkflowState.model.js
+++ b/src/model/WorkflowState.model.js
@@ -19,5 +19,4 @@ import { Enumify } from 'enumify'
 
 import {
-  mdiHelpCircle,
   mdiPauseCircle,
   mdiPlayCircle,
@@ -34,5 +33,4 @@ export class WorkflowState extends Enumify {
   static STOPPING = new WorkflowState('stopping', mdiSkipNextCircle)
   static STOPPED = new WorkflowState('stopped', mdiStopCircle)
-  static ERROR = new WorkflowState('error', mdiHelpCircle)
   static _ = this.closeEnum()

diff --git a/src/components/cylc/gscan/WorkflowIcon.vue b/src/components/cylc/gscan/WorkflowIcon.vue
index 8eba1138..23ff8c18 100644
--- a/src/components/cylc/gscan/WorkflowIcon.vue
+++ b/src/components/cylc/gscan/WorkflowIcon.vue
@@ -24,4 +24,5 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 <script>
 import WorkflowState from '@/model/WorkflowState.model'
+import { mdiHelpCircle } from '@mdi/js'
 
 /**
@@ -42,14 +43,9 @@ export default {
      * Return the workflow icon, based on the status prop. If the state is
      * not valid, we return a pre-defined error icon.
-     * @returns {String} - status, one of the WorkflowState enum values
+     * @returns {string} - status, one of the WorkflowState enum values
      */
     getIcon () {
-      // TBD: enumValueOf returned the wrong value?
-      const state = [...WorkflowState.enumValues]
-        .find(state => state.name === this.status)
-      if (!state || state.length === 0) {
-        return WorkflowState.ERROR.icon
-      }
-      return state.icon
+      const state = WorkflowState.enumValues.find(state => state.name === this.status)
+      return state?.icon || mdiHelpCircle
     }
   }

@oliver-sanders
Copy link
Member Author

If that's the only reference to ERROR in the UI code, then that would be my preferred solution. There has never been an error state cylc-flow side.

Do you want to raise that diff?

@oliver-sanders
Copy link
Member Author

Superseded by #1563

@oliver-sanders oliver-sanders removed this from the 2.3.0 milestone Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants