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

Generalize provider error handling to processes #1495

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

totycro
Copy link
Contributor

@totycro totycro commented Jan 15, 2024

Overview

This allows triggering errors from a processor with specific http status code, ogc exception code and a custom message.

This is very useful the processor realizes that the input parameters don't make sense or are not allowed, in which case it can supply a descriptive error message and an http status code different from 500.

Related issue / discussion

Recent PR introducing the base behavior for providers: #1392

Additional information

Dependency policy (RFC2)

  • I have ensured that this PR meets RFC2 requirements

Updates to public demo

Contributions and licensing

(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)

  • I'd like to contribute [feature X|bugfix Y|docs|something else] to pygeoapi. I confirm that my contributions to pygeoapi will be compatible with the pygeoapi license guidelines at the time of contribution
  • I have already previously agreed to the pygeoapi Contributions and Licensing Guidelines

This allows triggering errors from a processor with specific
http status code, ogc exception code and a custom message.

This is very useful the processor realizes that the input parameters
don't make sense or are not allowed, in which case it can supply a
descriptive error message and an http status code different from 500.
@ricardogsilva
Copy link
Member

Hi @totycro

This is not a review, just an opinion, so take it with a grain of salt 😉

In my opinion, a processor should not be able to decide what the overall status code of the server's response is going to be, as that is beyond its responsibilities. In fact, I think the processor should not need to know anything about HTTP - no status code, no headers, nothing. The reasoning for this is that it seems to create unnecessary coupling between the running process and an eventual HTTP response - We are able to achieve the same result without having the processor dealing with HTTP stuff.

Each job ( i.e. a running process) ought to generate:

  • An execution status - i.e. accepted, running, etc;
  • Execution details - this would either some progress message, a final success message, or a description of an eventual error;
  • A set of results for when execution is successful (either the outputs or just a list of locations to where they can be retrieved);
  • A set of additional information details, or metadata about the execution if you will - these would likely be included as headers in an eventual HTTP response;

This information is to be stored by the process manager somewhere so that it may be made available either immediately after execution, or later, at the client's request (depending on whether the execution mode as sync or async).

It would then be the responsibility of the pygeoapi.API class to ask the process manager for a job's details and then transform them into a suitable HTTP response. This includes generating responses dealing with errors.

Broadly speaking, in terms of errors, either:

  • The client made some bad request and the process is not even able to start running - pygeoapi would return with some 4xx status code immediately. We can probably add some additional exception classes to be raised by the code if the existing one does not suffice;

  • The server encountered some error while running the process. This means the job information would have been created and stored by the process manager, which API would be able to retrieve in order to get relevant details about the error:

    • if execution mode is sync, pygeoapi would return with 5xx status code immediately
    • if execution mode is async, the job status page would still reply with a 200 status code, but the job properties would clearly state the error state and description (this is basically what the standard says)

This conceptualization does not introduce any additional components to pygeoapi, as they already exist - granted they may not be working exactly like this, as I think there is some refactoring to be done in order to reach parity with the OAProc v1.0.0 standard.

I do think there is ample room for improvement of process execution and error handling, it is just that it seems this PR is setting up the path for moving things in a different direction than what I perceive as the goal, which would be to have as little coupling between process execution and HTTP request/responses as possible.

@tomkralidis tomkralidis merged commit 1da681b into geopython:master Jan 15, 2024
4 checks passed
@totycro totycro deleted the process-error-handling branch January 15, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants