Skip to content

Commit

Permalink
Merge pull request #17593 from jmchilton/invocation_cleanup
Browse files Browse the repository at this point in the history
Modernization and cleanup of job state related client code.
  • Loading branch information
dannon authored Mar 12, 2024
2 parents 728bbeb + bf78004 commit 17d015e
Show file tree
Hide file tree
Showing 14 changed files with 250 additions and 487 deletions.
12 changes: 7 additions & 5 deletions client/src/api/invocations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import axios from "axios";

import { getAppRoot } from "@/onload";

import { ApiResponse, fetcher } from "./schema";
import { ApiResponse, components, fetcher } from "./schema";

export type WorkflowInvocationElementView = components["schemas"]["WorkflowInvocationElementView"];
export type WorkflowInvocationCollectionView = components["schemas"]["WorkflowInvocationCollectionView"];
export type InvocationJobsSummary = components["schemas"]["InvocationJobsResponse"];
export type InvocationStep = components["schemas"]["InvocationStep"];

export const invocationsFetcher = fetcher.path("/api/invocations").method("get").create();

// TODO: Replace these interfaces with real schema models after https://github.com/galaxyproject/galaxy/pull/16707 is merged
export interface WorkflowInvocation {
id: string;
}
export type WorkflowInvocation = WorkflowInvocationElementView | WorkflowInvocationCollectionView;

export interface WorkflowInvocationJobsSummary {
id: string;
Expand Down
4 changes: 2 additions & 2 deletions client/src/components/JobInformation/JobInformation.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import CopyToClipboard from "components/CopyToClipboard";
import HelpText from "components/Help/HelpText";
import { JobDetailsProvider } from "components/providers/JobProvider";
import UtcDate from "components/UtcDate";
import { NON_TERMINAL_STATES } from "components/WorkflowInvocationState/util";
import { formatDuration, intervalToDuration } from "date-fns";
import JOB_STATES_MODEL from "utils/job-states-model";
import { computed, ref } from "vue";
import { invocationForJob } from "@/api/invocations";
Expand All @@ -30,7 +30,7 @@ const runTime = computed(() =>
formatDuration(intervalToDuration({ start: new Date(job.value.create_time), end: new Date(job.value.update_time) }))
);
const jobIsTerminal = computed(() => job.value && !JOB_STATES_MODEL.NON_TERMINAL_STATES.includes(job.value.state));
const jobIsTerminal = computed(() => job.value && !NON_TERMINAL_STATES.includes(job.value.state));
const routeToInvocation = computed(() => `/workflows/invocations/${invocationId.value}`);
Expand Down
64 changes: 0 additions & 64 deletions client/src/components/JobStates/CollectionJobStates.vue

This file was deleted.

47 changes: 0 additions & 47 deletions client/src/components/JobStates/mixin.js

This file was deleted.

6 changes: 3 additions & 3 deletions client/src/components/JobStates/wait.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import axios from "axios";
import { ERROR_STATES, NON_TERMINAL_STATES } from "components/WorkflowInvocationState/util";
import { getAppRoot } from "onload/loadConfig";
import JOB_STATES_MODEL from "utils/job-states-model";

export function waitOnJob(jobId, onStateUpdate = null, interval = 1000) {
// full=true to capture standard error on last iteration for building
Expand All @@ -14,9 +14,9 @@ export function waitOnJob(jobId, onStateUpdate = null, interval = 1000) {
if (onStateUpdate !== null) {
onStateUpdate(state);
}
if (JOB_STATES_MODEL.NON_TERMINAL_STATES.indexOf(state) !== -1) {
if (NON_TERMINAL_STATES.indexOf(state) !== -1) {
setTimeout(checkCondition, interval, resolve, reject);
} else if (JOB_STATES_MODEL.ERROR_STATES.indexOf(state) !== -1) {
} else if (ERROR_STATES.indexOf(state) !== -1) {
reject(jobResponse);
} else {
resolve(jobResponse);
Expand Down
6 changes: 3 additions & 3 deletions client/src/components/RuleCollectionBuilder.vue
Original file line number Diff line number Diff line change
Expand Up @@ -585,11 +585,11 @@ import SaveRules from "components/RuleBuilder/SaveRules";
import StateDiv from "components/RuleBuilder/StateDiv";
import Select2 from "components/Select2";
import UploadUtils from "components/Upload/utils";
import { ERROR_STATES, NON_TERMINAL_STATES } from "components/WorkflowInvocationState/util";
import $ from "jquery";
import { getAppRoot } from "onload/loadConfig";
import _ from "underscore";
import { refreshContentsWrapper } from "utils/data";
import JobStatesModel from "utils/job-states-model";
import _l from "utils/localization";
import Vue from "vue";
Expand Down Expand Up @@ -1337,9 +1337,9 @@ export default {
const handleJobShow = (jobResponse) => {
const state = jobResponse.data.state;
this.waitingJobState = state;
if (JobStatesModel.NON_TERMINAL_STATES.indexOf(state) !== -1) {
if (NON_TERMINAL_STATES.indexOf(state) !== -1) {
setTimeout(doJobCheck, 1000);
} else if (JobStatesModel.ERROR_STATES.indexOf(state) !== -1) {
} else if (ERROR_STATES.indexOf(state) !== -1) {
this.state = "error";
this.errorMessage =
"Unknown error encountered while running your upload job, this could be a server issue or a problem with the upload definition.";
Expand Down
3 changes: 2 additions & 1 deletion client/src/components/Workflow/test/json/invocation.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@
"input_step_parameters": {},
"outputs": {},
"output_collections": {},
"output_values": {}
"output_values": {},
"messages": []
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { setActivePinia } from "pinia";
import { getLocalVue } from "tests/jest/helpers";

import type { WorkflowInvocation } from "@/api/invocations";
import JOB_STATES_MODEL from "@/utils/job-states-model";

import invocationData from "../Workflow/test/json/invocation.json";

Expand Down Expand Up @@ -34,7 +33,7 @@ async function mountWorkflowInvocationState(invocation: WorkflowInvocation | nul
},
computed: {
invocation: () => invocation,
jobStatesSummary: () => new JOB_STATES_MODEL.JobStatesSummary(invocationJobsSummaryById),
jobStatesSummary: () => invocationJobsSummaryById,
},
pinia,
localVue,
Expand All @@ -45,7 +44,7 @@ async function mountWorkflowInvocationState(invocation: WorkflowInvocation | nul

describe("WorkflowInvocationState.vue", () => {
it("determines that invocation and job states are terminal with terminal invocation", async () => {
const wrapper = await mountWorkflowInvocationState(invocationData);
const wrapper = await mountWorkflowInvocationState(invocationData as WorkflowInvocation);
expect(isInvocationAndJobTerminal(wrapper)).toBe(true);
});

Expand All @@ -58,7 +57,7 @@ describe("WorkflowInvocationState.vue", () => {
const invocation = {
...invocationData,
state: "new",
};
} as WorkflowInvocation;
const wrapper = await mountWorkflowInvocationState(invocation);
expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@
</b-alert>
</template>
<script>
import mixin from "components/JobStates/mixin";
import LoadingSpan from "components/LoadingSpan";
import JOB_STATES_MODEL from "utils/job-states-model";
import { useInvocationStore } from "@/stores/invocationStore";
import { cancelWorkflowScheduling } from "./services";
import { isTerminal, jobCount } from "./util";
import WorkflowInvocationDetails from "./WorkflowInvocationDetails.vue";
import WorkflowInvocationExportOptions from "./WorkflowInvocationExportOptions.vue";
Expand All @@ -49,7 +48,6 @@ export default {
WorkflowInvocationDetails,
WorkflowInvocationExportOptions,
},
mixins: [mixin],
props: {
invocationId: {
type: String,
Expand Down Expand Up @@ -91,15 +89,15 @@ export default {
);
},
jobStatesTerminal: function () {
if (this.invocationSchedulingTerminal && this.JobStatesSummary?.jobCount === 0) {
if (this.invocationSchedulingTerminal && jobCount(this.jobStatesSummary) === 0) {
// no jobs for this invocation (think subworkflow or just inputs)
return true;
}
return this.jobStatesSummary && this.jobStatesSummary.terminal();
return this.jobStatesSummary && isTerminal(this.jobStatesSummary);
},
jobStatesSummary() {
const jobsSummary = this.invocationStore.getInvocationJobsSummaryById(this.invocationId);
return !jobsSummary ? null : new JOB_STATES_MODEL.JobStatesSummary(jobsSummary);
return !jobsSummary ? null : jobsSummary;
},
},
created: function () {
Expand Down
Loading

0 comments on commit 17d015e

Please sign in to comment.