Skip to content

Commit

Permalink
Merge pull request #794 from crim-ca/status-successful
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault authored Feb 6, 2025
2 parents 9d8c0a4 + cd59034 commit 2c08b75
Show file tree
Hide file tree
Showing 28 changed files with 266 additions and 168 deletions.
1 change: 1 addition & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ disable=C0111,missing-docstring,
R0901,too-many-ancestors,
R0902,too-many-instance-attributes,
R0904,too-many-public-methods,
R0911,too-many-return-statements,
R0912,too-many-branches,
R0913,too-many-arguments,
R0914,too-many-locals,
Expand Down
7 changes: 6 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ Changes

Changes:
--------
- No change.
- Replace ``succeeded`` status by ``successful`` everywhere where applicable (as originally defined by OGC API v1),
to align with reversal of the proposed draft name, aligning between both v1 and v2 of `OGC API - Processes`
(relates to `opengeospatial/ogcapi-processes#483 <https://github.com/opengeospatial/ogcapi-processes/pull/483>`_).
- Modify `Job` ``subscribers`` definition to employ the normalized ``weaver.status.StatusCategory`` instead
of ``weaver.status.Status`` as mapping keys, such that email and callback notifications are unified under
a common naming convention regardless of the resolved ``weaver.status.StatusCompliant`` representation.

Fixes:
------
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ conda-base: ## obtain and install a missing conda distribution
echo "Make sure to add '$(CONDA_BIN_DIR)' to your PATH variable in '~/.bashrc'.")

.PHONY: conda-clean
clean-clean: ## remove the conda environment
conda-clean: ## remove the conda environment
@echo "Removing conda env '$(CONDA_ENV)'"
@-test -d "$(CONDA_ENV_PATH)" && "$(CONDA_BIN)" remove -n "$(CONDA_ENV)" --yes --all

Expand Down Expand Up @@ -344,8 +344,8 @@ clean-docs-dirs: ## remove documentation artifacts (minimal)
clean-src: ## remove all *.pyc files
@echo "Removing python artifacts..."
@-find "$(APP_ROOT)" -type f -name "*.pyc" -exec rm {} \;
@-rm -rf ./build
@-rm -rf ./src
@-rm -rf "$(APP_ROOT)/build"
@-rm -rf "$(APP_ROOT)/src"

.PHONY: clean-test
clean-test: ## remove files created by tests and coverage analysis
Expand Down
4 changes: 2 additions & 2 deletions docs/examples/job_status_ogcapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"processID": "example-process",
"providerID": null,
"type": "process",
"status": "succeeded",
"message": "Job succeeded.",
"status": "successful",
"message": "Job successful.",
"created": "2024-10-02T14:21:12.380000+00:00",
"started": "2024-10-02T14:21:12.990000+00:00",
"finished": "2024-10-02T14:21:23.629000+00:00",
Expand Down
4 changes: 4 additions & 0 deletions docs/source/appendix.rst
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,10 @@ Glossary
.. seealso::
:ref:`quotation_estimator_model`

openEO
| Open Earth Observation
| Cloud backend :term:`API` initiative for unified Earth Observation, as described by :ref:`openEO`.
OpenSearch
Protocol of lookup and retrieval of remotely stored files.
Please refer to :ref:`OpenSearch Data Source` for details.
Expand Down
2 changes: 1 addition & 1 deletion docs/source/processes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ Once the :term:`Job` is submitted, its status should initially switch to ``accep
status will change to ``started`` for preparation steps (i.e.: allocation resources, retrieving required
parametrization details, etc.), followed by ``running`` when effectively reaching the execution step of the underlying
:term:`Application Package` operation. This status will remain as such until the operation completes, either with
``succeeded`` or ``failed`` status.
``successful`` or ``failed`` status.

At any moment during |asynchronous|_ execution, the :term:`Job` status can be requested using |status-req|_. Note that
depending on the timing at which the user executes this request and the availability of task workers, it could be
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ def test_jsonarray2netcdf_execute_sync(self):
assert resp.content_type == ContentType.APP_JSON
for field in ["status", "created", "finished", "duration", "progress"]:
assert field in resp.json
assert resp.json["status"] == Status.SUCCEEDED
assert resp.json["status"] == Status.SUCCESSFUL
assert resp.json["progress"] == 100

out_url = f"{job_url}/results"
Expand Down
66 changes: 37 additions & 29 deletions tests/functional/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ def test_execute_manual_monitor_status_and_download_results(self):
result = mocked_sub_requests(self.app, self.client.monitor, job_id, timeout=5, interval=1)
assert result.success, result.text
assert "undefined" not in result.message
assert result.body.get("status") == Status.SUCCEEDED
assert result.body.get("status") == Status.SUCCESSFUL
links = result.body.get("links")
assert isinstance(links, list)
assert len([_link for _link in links if _link["rel"].endswith("results")]) == 1
Expand Down Expand Up @@ -684,8 +684,8 @@ def test_execute_subscribers(self):
}
}

# order important, expect status 'started' (in-progress) to occur before 'succeeded'
# call for 'failed' should never happen since 'succeeded' expected, as validated by above method
# order important, expect status 'started' (in-progress) to occur before 'successful'
# call for 'failed' should never happen since 'successful' expected, as validated by above method
assert mocked_requests.call_count == 2, "Should not have called both failed/success callback requests"
assert mocked_requests.call_args_list[0].args == ("POST", subscribers["inProgressUri"])
assert mocked_requests.call_args_list[0].kwargs["json"]["status"] in running_statuses # status JSON
Expand All @@ -698,7 +698,7 @@ def test_execute_subscribers(self):
assert mocked_emails.call_args_list[0].args[:2] == (None, subscribers["inProgressEmail"])
assert f"Job {test_proc_byte} Started".encode() in mocked_emails.call_args_list[0].args[-1]
assert mocked_emails.call_args_list[1].args[:2] == (None, subscribers["successEmail"])
assert f"Job {test_proc_byte} Succeeded".encode() in mocked_emails.call_args_list[1].args[-1]
assert f"Job {test_proc_byte} Successful".encode() in mocked_emails.call_args_list[1].args[-1]

# NOTE:
# For all below '<>_auto_resolve_vault' test cases, the local file referenced in the Execute request body
Expand Down Expand Up @@ -834,17 +834,19 @@ def test_update_job(self):

result = mocked_sub_requests(self.app, self.client.status, job_id)
assert result.success
assert isinstance(result.body, dict)
assert result.body["title"] == "Random Title"

result = mocked_sub_requests(self.app, self.client.inputs, job_id)
assert result.success
assert isinstance(result.body, dict)
assert result.body["inputs"] == {"message": "new message"}
assert result.body["outputs"] == {"output": {}}
assert result.body["headers"]["Prefer"] == f"return={ExecuteReturnPreference.REPRESENTATION}; respond-async"

@mocked_dismiss_process()
def test_dismiss(self):
for status in [Status.ACCEPTED, Status.FAILED, Status.RUNNING, Status.SUCCEEDED]:
for status in [Status.ACCEPTED, Status.FAILED, Status.RUNNING, Status.SUCCESSFUL]:
proc = self.test_process["Echo"]
job = self.job_store.save_job(task_id="12345678-1111-2222-3333-111122223333", process=proc)
job.status = status
Expand All @@ -859,7 +861,7 @@ def test_jobs_search_multi_status(self):
job1 = self.job_store.save_job(task_id=uuid.uuid4(), process=proc, access=Visibility.PUBLIC)
job2 = self.job_store.save_job(task_id=uuid.uuid4(), process=proc, access=Visibility.PUBLIC)
job3 = self.job_store.save_job(task_id=uuid.uuid4(), process=proc, access=Visibility.PUBLIC)
job1.status = Status.SUCCEEDED
job1.status = Status.SUCCESSFUL
job2.status = Status.FAILED
job3.status = Status.RUNNING
job1 = self.job_store.update_job(job1)
Expand All @@ -868,10 +870,10 @@ def test_jobs_search_multi_status(self):
jobs = [job1, job2, job3]

for test_status, job_expect in [
(Status.SUCCEEDED, [job1]),
([Status.SUCCEEDED], [job1]),
([Status.SUCCEEDED, Status.RUNNING], [job1, job3]),
(f"{Status.SUCCEEDED},{Status.RUNNING}", [job1, job3]),
(Status.SUCCESSFUL, [job1]),
([Status.SUCCESSFUL], [job1]),
([Status.SUCCESSFUL, Status.RUNNING], [job1, job3]),
(f"{Status.SUCCESSFUL},{Status.RUNNING}", [job1, job3]),
(StatusCategory.FINISHED, [job1, job2]),
(StatusCategory.FINISHED.value, [job1, job2]),
([StatusCategory.FINISHED], [job1, job2]),
Expand All @@ -893,7 +895,7 @@ def setUp(self):
job = self.job_store.save_job(
task_id="12345678-1111-2222-3333-111122223333", process="fake-process", access=Visibility.PUBLIC
)
job.status = Status.SUCCEEDED
job.status = Status.SUCCESSFUL
self.test_job = self.job_store.update_job(job)

def test_help_operations(self):
Expand Down Expand Up @@ -1612,7 +1614,7 @@ def test_execute_inputs_capture(self):
entrypoint=weaver_cli,
only_local=True,
)
assert any(f"\"status\": \"{Status.SUCCEEDED}\"" in line for line in lines)
assert any(f"\"status\": \"{Status.SUCCESSFUL}\"" in line for line in lines)

def test_execute_manual_monitor(self):
proc = self.test_process["Echo"]
Expand Down Expand Up @@ -1656,7 +1658,7 @@ def test_execute_manual_monitor(self):
)

assert any(f"\"jobID\": \"{job_id}\"" in line for line in lines)
assert any(f"\"status\": \"{Status.SUCCEEDED}\"" in line for line in lines)
assert any(f"\"status\": \"{Status.SUCCESSFUL}\"" in line for line in lines)
assert any(f"\"href\": \"{job_ref}/results\"" in line for line in lines)
assert any("\"rel\": \"http://www.opengis.net/def/rel/ogc/1.0/results\"" in line for line in lines)

Expand All @@ -1683,7 +1685,7 @@ def test_execute_auto_monitor(self):
only_local=True,
)
assert any("\"jobID\": \"" in line for line in lines) # don't care value, self-handled
assert any(f"\"status\": \"{Status.SUCCEEDED}\"" in line for line in lines)
assert any(f"\"status\": \"{Status.SUCCESSFUL}\"" in line for line in lines)
assert any("\"rel\": \"http://www.opengis.net/def/rel/ogc/1.0/results\"" in line for line in lines)

def test_execute_result_by_reference(self):
Expand Down Expand Up @@ -1719,7 +1721,7 @@ def test_execute_result_by_reference(self):
only_local=True,
)
assert any(line.startswith("jobID: ") for line in lines[:2]) # don't care value, self-handled
assert any(f"status: {Status.SUCCEEDED}" in line for line in lines)
assert any(f"status: {Status.SUCCESSFUL}" in line for line in lines)
for line in lines:
if line.startswith("jobID: "):
job_id = line.split(":")[-1].strip()
Expand Down Expand Up @@ -1823,7 +1825,7 @@ def test_execute_output_context(self, cli_options, expect_output_context):
entrypoint=weaver_cli,
only_local=True,
)
assert any(f"status: {Status.SUCCEEDED}" in line for line in lines)
assert any(f"status: {Status.SUCCESSFUL}" in line for line in lines)
job_id = None
for line in lines:
if line.startswith("jobID: "):
Expand Down Expand Up @@ -1906,16 +1908,22 @@ def test_execute_subscriber_options(self):
only_local=True,
)
data = json.loads(lines[0])
assert data["status"] == Status.SUCCEEDED
assert data["status"] == Status.SUCCESSFUL

job = self.job_store.fetch_by_id(data["jobID"])
# to properly compare, we must decrypt emails (encrypt is not deterministic on multiple calls)
subs = copy.deepcopy(job.subscribers)
for sub, email in subs["emails"].items():
subs["emails"][sub] = decrypt_email(email, self.settings)
assert subs == {
"callbacks": {Status.STARTED: test_callback_started, Status.SUCCEEDED: test_callback_success},
"emails": {Status.STARTED: test_email_started, Status.FAILED: test_email_failed},
"callbacks": {
StatusCategory.RUNNING.value.lower(): test_callback_started,
StatusCategory.SUCCESS.value.lower(): test_callback_success,
},
"emails": {
StatusCategory.RUNNING.value.lower(): test_email_started,
StatusCategory.FAILED.value.lower(): test_email_failed,
},
}, "Job subscribers should be as submitted, after combining CLI options, without extra or missing ones."

def test_execute_help_details(self):
Expand Down Expand Up @@ -1991,7 +1999,7 @@ def test_jobs_no_links_limit_status_filters(self):
# "weaver",
"jobs",
"-u", self.url,
"-S", Status.SUCCEEDED,
"-S", Status.SUCCESSFUL,
"-N", 1,
"-nL",
],
Expand All @@ -2017,7 +2025,7 @@ def test_jobs_no_links_nested_detail(self):
# "weaver",
"jobs",
"-u", self.url,
"-S", Status.SUCCEEDED,
"-S", Status.SUCCESSFUL,
"-D", # when details active, each job lists its own links
"-nL", # unless links are requested to be removed (top-most and nested ones)
],
Expand All @@ -2037,7 +2045,7 @@ def test_jobs_no_links_nested_detail(self):
def test_jobs_filter_status_multi(self):
self.job_store.clear_jobs()
job = self.job_store.save_job(task_id=uuid.uuid4(), process="test-process", access=Visibility.PUBLIC)
job.status = Status.SUCCEEDED
job.status = Status.SUCCESSFUL
job_s = self.job_store.update_job(job)
job = self.job_store.save_job(task_id=uuid.uuid4(), process="test-process", access=Visibility.PUBLIC)
job.status = Status.FAILED
Expand All @@ -2052,7 +2060,7 @@ def test_jobs_filter_status_multi(self):
# "weaver",
"jobs",
"-u", self.url,
"-S", Status.SUCCEEDED, Status.ACCEPTED,
"-S", Status.SUCCESSFUL, Status.ACCEPTED,
"-D",
"-nL", # unless links are requested to be removed (top-most and nested ones)
],
Expand All @@ -2067,9 +2075,9 @@ def test_jobs_filter_status_multi(self):
assert isinstance(body["jobs"], list)
assert len(body["jobs"])
assert not any(_job["jobID"] == str(job_f.uuid) for _job in body["jobs"])
assert all(job["status"] in [Status.SUCCEEDED, Status.ACCEPTED] for job in body["jobs"])
assert all(job["status"] in [Status.SUCCESSFUL, Status.ACCEPTED] for job in body["jobs"])
jobs_accept = list(filter(lambda _job: _job["status"] == Status.ACCEPTED, body["jobs"]))
jobs_success = list(filter(lambda _job: _job["status"] == Status.SUCCEEDED, body["jobs"]))
jobs_success = list(filter(lambda _job: _job["status"] == Status.SUCCESSFUL, body["jobs"]))
assert len(jobs_accept) == 1 and jobs_accept[0]["jobID"] == str(job_a.uuid)
assert len(jobs_success) == 1 and jobs_success[0]["jobID"] == str(job_s.uuid)

Expand Down Expand Up @@ -2339,7 +2347,7 @@ def test_job_logs(self):
job = self.job_store.save_job(task_id=uuid.uuid4(), process="test-process", access=Visibility.PUBLIC)
job.save_log(message="test start", progress=0, status=Status.ACCEPTED)
job.save_log(message="test run", progress=50, status=Status.RUNNING)
job.save_log(message="test done", progress=100, status=Status.SUCCEEDED)
job.save_log(message="test done", progress=100, status=Status.SUCCESSFUL)
job = self.job_store.update_job(job)

lines = mocked_sub_requests(
Expand All @@ -2358,7 +2366,7 @@ def test_job_logs(self):
assert lines[0] == "["
assert f"0% {Status.ACCEPTED}" in lines[1]
assert f"50% {Status.RUNNING}" in lines[2]
assert f"100% {Status.SUCCEEDED}" in lines[3]
assert f"100% {Status.SUCCESSFUL}" in lines[3]
assert lines[4] == "]"

def test_job_exceptions(self):
Expand Down Expand Up @@ -2394,7 +2402,7 @@ def test_job_exceptions(self):
def test_job_statistics(self):
job = self.job_store.save_job(task_id=uuid.uuid4(), process="test-process", access=Visibility.PUBLIC)
job.statistics = resources.load_example("job_statistics.json")
job.status = Status.SUCCEEDED # error if not completed
job.status = Status.SUCCESSFUL # error if not completed
job = self.job_store.update_job(job)

lines = mocked_sub_requests(
Expand Down Expand Up @@ -2488,7 +2496,7 @@ def test_execute_remote_input(self):
entrypoint=weaver_cli,
only_local=True,
)
assert any(f"\"status\": \"{Status.SUCCEEDED}\"" in line for line in lines)
assert any(f"\"status\": \"{Status.SUCCESSFUL}\"" in line for line in lines)


class TestWeaverClientAuthBase(TestWeaverClientBase):
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/test_job_provenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def test_job_prov_info_not_acceptable(self):
job = self.job_store.save_job(
"test",
process=self.proc_id,
status=Status.SUCCEEDED
status=Status.SUCCESSFUL
)
prov_url = job.prov_url(self.settings)
headers = self.json_headers # note: this is the test, while only plain text is supported
Expand Down Expand Up @@ -229,7 +229,7 @@ def test_job_prov_data_generated_missing(self):
job = self.job_store.save_job(
"test",
process=self.proc_id,
status=Status.SUCCEEDED
status=Status.SUCCESSFUL
)
prov_url = job.prov_url(self.settings)
resp = self.app.get(prov_url, headers=self.json_headers, expect_errors=True)
Expand All @@ -244,7 +244,7 @@ def test_job_prov_data_dynamic_missing(self):
job = self.job_store.save_job(
"test",
process=self.proc_id,
status=Status.SUCCEEDED
status=Status.SUCCESSFUL
)
prov_url = job.prov_url(self.settings)
headers = {"Accept": ContentType.TEXT_PLAIN}
Expand Down
4 changes: 2 additions & 2 deletions tests/functional/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ def validate_test_job_execution(self, job_location_url, user_headers=None, user_
lambda: timeout_running > 0,
message=(
"Maximum timeout reached for job execution test. "
f"Expected job status change from '{Status.RUNNING}' to '{Status.SUCCEEDED}' "
f"Expected job status change from '{Status.RUNNING}' to '{Status.SUCCESSFUL}' "
f"within {self.WEAVER_TEST_JOB_RUNNING_MAX_TIMEOUT}s since first '{Status.RUNNING}'."
)
)
Expand All @@ -1005,7 +1005,7 @@ def validate_test_job_execution(self, job_location_url, user_headers=None, user_
time.sleep(self.WEAVER_TEST_JOB_GET_STATUS_INTERVAL)
continue
if status in JOB_STATUS_CATEGORIES[StatusCategory.FINISHED]:
failed = status != Status.SUCCEEDED
failed = status not in JOB_STATUS_CATEGORIES[StatusCategory.SUCCESS]
logs, details = self.try_retrieve_logs(job_location_url, detailed_results=not failed)
self.assert_test(
lambda: not failed,
Expand Down
Loading

0 comments on commit 2c08b75

Please sign in to comment.