From 9d176592342223d205c95fc4731441713bfd0454 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 4 Mar 2022 15:46:59 +0100 Subject: [PATCH 01/34] Backport SingleQueryProvider improvements Use ``timeout``, not interval, bump to 3s, make stoppable Depending on the response time using setInterval can lead to a pileup of requests, see https://developer.mozilla.org/en-US/docs/Web/API/setInterval#ensure_that_execution_duration_is_shorter_than_interval_frequency. I've also bumped up the 500 ms interval to 3000ms. 500 ms is too low and will cause high server load. This also adds an optional `stopRefresh` function that allows the caller to determine when to stop polling, based on the returned response, and emits a `update:result` event that callers can use to update local data. --- .../DatasetInformation/DatasetError.test.js | 1 - .../DatasetInformation/DatasetError.vue | 2 +- .../DatasetInformation.test.js | 2 +- .../DatasetInformation/DatasetInformation.vue | 2 +- .../src/components/Details/DatasetDetails.vue | 2 +- .../DatasetCollectionContents.vue | 2 +- .../JobInformation/JobInformation.vue | 25 ++++- .../JobParameters/JobParameters.test.js | 2 +- .../DisplayApplications.test.js | 1 - .../Visualizations/DisplayApplications.vue | 2 +- .../providers/SingleQueryProvider.js | 95 +++++++++++++------ 11 files changed, 93 insertions(+), 43 deletions(-) diff --git a/client/src/components/DatasetInformation/DatasetError.test.js b/client/src/components/DatasetInformation/DatasetError.test.js index 1d1ccfa44f3..39c8b513a29 100644 --- a/client/src/components/DatasetInformation/DatasetError.test.js +++ b/client/src/components/DatasetInformation/DatasetError.test.js @@ -29,7 +29,6 @@ function buildWrapper(has_duplicate_inputs = true, has_empty_inputs = true, user result: { has_duplicate_inputs: has_duplicate_inputs, has_empty_inputs: has_empty_inputs }, }), DatasetProvider: MockProvider({ - resultLabel: "item", result: { id: "dataset_id", creating_job: "creating_job" }, }), FontAwesomeIcon: false, diff --git a/client/src/components/DatasetInformation/DatasetError.vue b/client/src/components/DatasetInformation/DatasetError.vue index 8ba10deb55b..92228b3b6b6 100644 --- a/client/src/components/DatasetInformation/DatasetError.vue +++ b/client/src/components/DatasetInformation/DatasetError.vue @@ -4,7 +4,7 @@

Failed to access Dataset details.

{{ errorMessage }} - + - +

Dataset Information

diff --git a/client/src/components/Details/DatasetDetails.vue b/client/src/components/Details/DatasetDetails.vue index 6d622e18068..f1ccf2920ab 100644 --- a/client/src/components/Details/DatasetDetails.vue +++ b/client/src/components/Details/DatasetDetails.vue @@ -2,7 +2,7 @@ + v-slot="{ result: dataset, loading: isDatasetLoading, error: datasetLoadingError }">
diff --git a/client/src/components/History/ContentItem/GenericContentItem/DatasetCollectionContents.vue b/client/src/components/History/ContentItem/GenericContentItem/DatasetCollectionContents.vue index 157e8280316..3b8f3c5d056 100644 --- a/client/src/components/History/ContentItem/GenericContentItem/DatasetCollectionContents.vue +++ b/client/src/components/History/ContentItem/GenericContentItem/DatasetCollectionContents.vue @@ -4,7 +4,7 @@ + v-slot="{ result: item, loading }">
diff --git a/client/src/components/JobInformation/JobInformation.vue b/client/src/components/JobInformation/JobInformation.vue index 47e4043fcb5..1e6da159a5a 100644 --- a/client/src/components/JobInformation/JobInformation.vue +++ b/client/src/components/JobInformation/JobInformation.vue @@ -72,7 +72,7 @@ diff --git a/client/src/components/JobParameters/JobParameters.test.js b/client/src/components/JobParameters/JobParameters.test.js index f76f88f223c..876c174231e 100644 --- a/client/src/components/JobParameters/JobParameters.test.js +++ b/client/src/components/JobParameters/JobParameters.test.js @@ -24,7 +24,7 @@ const DatasetProvider = { render() { return this.$scopedSlots.default({ loading: false, - item: raw, + result: raw, }); }, }; diff --git a/client/src/components/Visualizations/DisplayApplications.test.js b/client/src/components/Visualizations/DisplayApplications.test.js index 045a9dd67a4..1cafa13934a 100644 --- a/client/src/components/Visualizations/DisplayApplications.test.js +++ b/client/src/components/Visualizations/DisplayApplications.test.js @@ -44,7 +44,6 @@ describe("DisplayApplications", () => { }, ], }, - resultLabel: "item", }), }, localVue, diff --git a/client/src/components/Visualizations/DisplayApplications.vue b/client/src/components/Visualizations/DisplayApplications.vue index f70946adc77..23131a19ee0 100644 --- a/client/src/components/Visualizations/DisplayApplications.vue +++ b/client/src/components/Visualizations/DisplayApplications.vue @@ -1,6 +1,6 @@ diff --git a/client/src/components/Upload/Default.vue b/client/src/components/Upload/Default.vue index cf9a0b98dc3..bf950d4f18a 100644 --- a/client/src/components/Upload/Default.vue +++ b/client/src/components/Upload/Default.vue @@ -33,7 +33,12 @@ @@ -37,10 +31,6 @@ export default { type: String, default: null, }, - disabled: { - type: Boolean, - default: false, - }, }, }; diff --git a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue index 6d8543f0e66..4718b9365e1 100644 --- a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue +++ b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue @@ -2,18 +2,9 @@
Workflow: {{ model.name }} - +
- - {{ errorMessage }} - - + Expand to full workflow form.
@@ -47,7 +38,6 @@ export default { }, data() { return { - errorMessage: null, formData: {}, inputTypes: {}, }; @@ -80,23 +70,12 @@ export default { }); return inputs; }, - runButtonDisabled() { - return this.errorMessage !== null; - }, }, methods: { - onValidation(validationError) { - this.errorMessage = validationError - ? "Please address the issues highlighted below before proceeding." - : null; - }, onChange(data) { this.formData = data; }, onExecute() { - if (this.errorMessage) { - return; - } const replacementParams = {}; const inputs = {}; for (const inputName in this.formData) { diff --git a/client/src/mvc/ui/ui-misc.js b/client/src/mvc/ui/ui-misc.js index d32c0cdc7da..f6a755145ab 100644 --- a/client/src/mvc/ui/ui-misc.js +++ b/client/src/mvc/ui/ui-misc.js @@ -86,8 +86,7 @@ export var Input = Backbone.View.extend({ input: "_onchange", }, value: function (new_val) { - new_val !== undefined && - this.model.set("value", new_val === null || typeof new_val === "string" ? new_val : ""); + new_val !== undefined && this.model.set("value", typeof new_val === "string" ? new_val : ""); return this.model.get("value"); }, render: function () { diff --git a/lib/galaxy/selenium/navigates_galaxy.py b/lib/galaxy/selenium/navigates_galaxy.py index 24a02d30dd5..cdedba69a33 100644 --- a/lib/galaxy/selenium/navigates_galaxy.py +++ b/lib/galaxy/selenium/navigates_galaxy.py @@ -1217,7 +1217,7 @@ class NavigatesGalaxy(HasDriver): self.send_enter(tag_area) def workflow_run_submit(self): - self.wait_for_and_click_selector("#run-workflow") + self.wait_for_and_click_selector("button.btn-primary") def workflow_create_new(self, annotation=None, clear_placeholder=False): self.workflow_index_open() diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 874fd2c2ef7..455d32e2b17 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -326,7 +326,11 @@ class SimpleTextToolParameter(ToolParameter): def to_json(self, value, app, use_security): """Convert a value to a string representation suitable for persisting""" - return unicodify(value) + if value is None: + rval = '' if not self.optional else None + else: + rval = unicodify(value) + return rval def get_initial_value(self, trans, other_values): return self.value diff --git a/lib/galaxy/tools/parameters/validation.py b/lib/galaxy/tools/parameters/validation.py index 3565e6cfe91..becd7e6e74e 100644 --- a/lib/galaxy/tools/parameters/validation.py +++ b/lib/galaxy/tools/parameters/validation.py @@ -737,9 +737,7 @@ class EmptyTextfieldValidator(Validator): return cls(message, negate) def validate(self, value, trans=None): - # Ideally the empty string validator should only check against an actual empty string, - # but the fill_defaults logic will store the result of `get_initial_value``, which is None. - super().validate(value not in ('', None)) + super().validate(value != '') class MetadataInFileColumnValidator(Validator): diff --git a/lib/galaxy/webapps/galaxy/controllers/workflow.py b/lib/galaxy/webapps/galaxy/controllers/workflow.py index 72f930a44b9..f82f8da245e 100644 --- a/lib/galaxy/webapps/galaxy/controllers/workflow.py +++ b/lib/galaxy/webapps/galaxy/controllers/workflow.py @@ -429,7 +429,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi stored = self.get_stored_workflow(trans, id) if new_annotation: # Sanitize annotation before adding it. - new_annotation = sanitize_html(new_annotation or '') + new_annotation = sanitize_html(new_annotation) self.add_item_annotation(trans.sa_session, trans.get_user(), stored, new_annotation) trans.sa_session.flush() return new_annotation @@ -572,7 +572,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi workflow.stored_workflow = stored_workflow stored_workflow.latest_workflow = workflow # Add annotation. - workflow_annotation = sanitize_html(workflow_annotation or '') + workflow_annotation = sanitize_html(workflow_annotation) self.add_item_annotation(trans.sa_session, trans.get_user(), stored_workflow, workflow_annotation) # Persist session = trans.sa_session @@ -598,7 +598,7 @@ class WorkflowController(BaseUIController, SharableMixin, UsesStoredWorkflowMixi workflow.stored_workflow = stored_workflow stored_workflow.latest_workflow = workflow # Add annotation. - workflow_annotation = sanitize_html(workflow_annotation or '') + workflow_annotation = sanitize_html(workflow_annotation) self.add_item_annotation(trans.sa_session, trans.get_user(), stored_workflow, workflow_annotation) # Persist diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 69087aab503..f4dcb681d13 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -865,9 +865,9 @@ class InputParameterModule(WorkflowModule): ) if param_type == "text": if parameter_type == "text": - text_default = parameter_def.get("default") + text_default = parameter_def.get("default") or "" else: - text_default = None + text_default = "" default_source["value"] = text_default input_default_value: Union[ TextToolParameter, diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index 0e2627eeff3..bf830e41064 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -89,17 +89,10 @@ def _normalize_inputs(steps, inputs, inputs_by): # start to ensure tool state is being preserved and loaded in a type safe way. assert isinstance(optional, bool) if not inputs_key and default_value is None and not optional: - message = f"Workflow cannot be run because an expected input step '{step.order_index}' ({step.label}) is not optional and no input." + message = f"Workflow cannot be run because an expected input step '{step.id}' ({step.label}) is not optional and no input." raise exceptions.MessageException(message) if inputs_key: - input_value = inputs[inputs_key] - if input_value is None and default_value is None and not optional: - # We've got a null value in a required parameter. - # I can't think of a situation where this should be valid, - # so let's fail early here - message = f"Workflow cannot be run because an expected input step '{step.order_index}' ({step.label}) is not optional and input is null." - raise exceptions.MessageException(message) - normalized_inputs[step.id] = input_value + normalized_inputs[step.id] = inputs[inputs_key] return normalized_inputs diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 0be1fb889af..decdf886286 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -2592,34 +2592,6 @@ text_input: jobs = self._history_jobs(history_id) assert len(jobs) == 1 - def test_run_with_required_text_parameter_not_provided_fails(self): - with self.dataset_populator.test_history() as history_id: - try: - self._run_workflow(""" -class: GalaxyWorkflow -inputs: - text_input: text -steps: [] -""", assert_ok=True, history_id=history_id) - except AssertionError as e: - assert '(text_input) is not optional' in str(e) - - def test_run_with_required_text_parameter_null_fails(self): - with self.dataset_populator.test_history() as history_id: - try: - self._run_workflow(""" -class: GalaxyWorkflow -inputs: - text_input: text -steps: [] -""", test_data=""" -text_input: - value: null - type: raw -""", assert_ok=True, history_id=history_id) - except AssertionError as e: - assert '(text_input) is not optional' in str(e) - def test_run_with_int_parameter(self): with self.dataset_populator.test_history() as history_id: failed = False diff --git a/lib/galaxy_test/selenium/test_workflow_run.py b/lib/galaxy_test/selenium/test_workflow_run.py index e955fc829ea..10887c2f24b 100644 --- a/lib/galaxy_test/selenium/test_workflow_run.py +++ b/lib/galaxy_test/selenium/test_workflow_run.py @@ -254,19 +254,6 @@ steps: history_id = self.current_history_id() self.workflow_populator.wait_for_history_workflows(history_id, expected_invocation_count=1) - @selenium_test - def test_simple_workflow_run_form_validation_required_text_param(self): - workflow_id = self.workflow_populator.upload_yaml_workflow(""" -class: GalaxyWorkflow -inputs: - text_input: text -steps: [] -""") - self.simplified_workflow_run_by_id(workflow_id) - run_button = self.components.workflow_run.run_workflow.wait_for_visible() - assert run_button.get_attribute('disabled') == 'true' - self.components.workflow_run.validation_error.wait_for_visible() - def open_in_workflow_run(self, yaml_content): name = self.workflow_upload_yaml_with_random_name(yaml_content) self.workflow_run_with_name(name) @@ -293,9 +280,6 @@ steps: [] self.workflow_index_search_for(name) self.workflow_click_option(".workflow-run") - def simplified_workflow_run_by_id(self, workflow_id): - self.get(f"workflows/run?id={workflow_id}&simplified_workflow_run_ui=prefer") - def _assert_has_3_lines_after_run(self, hid): if self.is_beta_history(): self.content_item_by_attributes(hid=hid, state="ok").wait_for_present()