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

clarify /req/core/job-results-async-raw-value-one #95

Closed
tomkralidis opened this issue Mar 5, 2024 · 6 comments
Closed

clarify /req/core/job-results-async-raw-value-one #95

tomkralidis opened this issue Mar 5, 2024 · 6 comments
Assignees
Projects
Milestone

Comments

@tomkralidis
Copy link

tomkralidis commented Mar 5, 2024

Against a local pygeoapi development instance, method testJobResultsAsyncRawValueOne fails with the following detail:

java.lang.AssertionError: No result (rel='http://www.opengis.net/def/rel/ogc/1.0/results') or monitor (rel='monitor') links were found in response. 

When the ETS queries a /jobs endpoint, some of the pygeoapi jobs do not have any links because the process failed to execute (so status=failed). In this case, the pygeoapi job does not emit any link array.

I am guessing pygeoapi is failing ETS at this spot.

The question then becomes: when looping through jobs, should the ETS inspect the job status of each job before determining whether a links array should exist with a link relation of "http://www.opengis.net/def/rel/ogc/1.0/results" ? Note that links is also optional in https://github.com/opengeospatial/ogcapi-processes/blob/master/openapi/schemas/processes-core/statusInfo.yaml

I can post a temporary public server/endpoint for a few hours if that helps.

@ghobona @gfenoy @jerstlouis

@jerstlouis
Copy link
Member

There are a few things that are somewhat ambiguous in version 1.0.

Therefore, the ETS for 1.0 should probably be forgiving due to the lack of clarity / requirements for 1.0.

For the upcoming version 1.1 (or 2.0), these things should already have been fixed in the latest draft for 1.1 (or 2.0) -- we should make sure that these things are now clear (at least as recommendations if needed to make 1.1 backward compatible, which the ETS could still warn upon these things missing).

In the case of a FAILED job, there should definitely NOT be any requirement to have results linked from the job status, but the monitor link (which was returned upon the 201 execution) should be still be available to retrieve information about the failed job.

cc. @pvretano

@gfenoy
Copy link
Contributor

gfenoy commented Mar 6, 2024

First, I'd like to point out that we have the same issue with this test with the ZOO-Project. Nevertheless, you may have noticed that this test is new because it is only available in the beta TeamEngine.

If we look into the source code of the function responsible for this test, we may notice that if the status code is not 200 then the function invoke the loopOverStatus method. This method is defined here. My understanding when reading this function is that there are two loops, the first one to check if there is a link with the following relation type http://www.opengis.net/def/rel/ogc/1.0/results, the second on the check if there is a link with the relation type status or monitor. All this looks good; nevertheless, the function then throws an AssertionException whatever happened in the loop and the value of hasMonitorOrResultLink. Consequently, even if the loop finds a results link or one with monitor, the function still throws the exception.

If I can have access for a few hours, I shall be able to debug what happens with your server instance in more detail.

On the other hand, I have set up this version of the ETS: http://145.239.195.35:1082/teamengine/ (you can use ogctest/ogctest to authenticate), including the fix from PR #96, for you to test your server instance. As you can see in the PR, the function now returns when a results link is found or after looping over the monitor link if there is no results.

@dstenger
Copy link
Contributor

dstenger commented Mar 6, 2024

Thanks for your input and the pull request. We will review it and come back to you.

@dstenger dstenger added this to To do in CITE via automation Mar 6, 2024
@dstenger dstenger moved this from To do to In progress in CITE Mar 6, 2024
@tomkralidis
Copy link
Author

Thanks @gfenoy. I've stood up an adhoc endpoint at http://demo.pygeoapi.io:8888. Testing against your CITE instance at http://145.239.195.35:1082 (using hello-world process) now results in 0 failures.

@tomkralidis
Copy link
Author

Update: given the above, the main pygeoapi demo now includes this change (temporary endpoint remove and now can tested via https://demo.pygeoapi.io/cite).

@dstenger dstenger added this to the 1.2 milestone Mar 21, 2024
@dstenger
Copy link
Contributor

#96 was merged.

CITE automation moved this from In progress to Done Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
CITE
  
Done
Development

No branches or pull requests

5 participants