idanmiara commented 3 years ago

Overview

This PR adds partial support for the OGC API - Processes / REST API and allows requests and responses in JSON format. I'd be happy to get a review. Full backwards compatibility was maintained to allow both XML and JSON input/output, the correct format is determinate by the mimetype/content-type and the accept headers (POST) and f argument in GET.

I've add a /processes and /jobs end points that default to JSON input/output. The original /wps endpoint defaults to XML as before.

I've yet to add an open-api/swagger interface, but I've tried to make it as compatible as possible to these APIs:

ZOO-Project WPS

52°North

Example JSON requests:

I've taken the internal JSON format and tried to make it a valid request (which now work).

Original XML request:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<wps:Execute service="WPS" version="1.0.0" xmlns:wps="http://www.opengis.net/wps/1.0.0" xmlns:ows="http://www.opengis.net/ows/1.1" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.opengis.net/wps/1.0.0 ../wpsExecute_request.xsd">
    <!-- <ows:Identifier>say_hello</ows:Identifier> -->

    <wps:ResponseForm>
        <wps:RawDataOutput>
          <ows:Identifier>output</ows:Identifier>
        </wps:RawDataOutput>
    </wps:ResponseForm>

    <wps:DataInputs>
        <wps:Input>
            <ows:Identifier>name</ows:Identifier>
            <wps:Data>
                <wps:LiteralData>Idan</wps:LiteralData>
            </wps:Data>
        </wps:Input>
    </wps:DataInputs>

</wps:Execute>

JSON request taken from internal pywps (which now accepted as a valid request):

{
    "operation": "execute",
    "version": "1.0.0",
    "language": "en-US",
    "identifier": "say_hello",
    "identifiers": null,
    "store_execute": "false",
    "status": "false",
    "lineage": "false",
    "inputs": {
        "name": [
            {
                "identifier": "name",
                "title": "Input name",
                "abstract": "",
                "keywords": [],
                "metadata": [],
                "type": "literal",
                "data_type": "string",
                "workdir": "D:\\dev\\gis\\wps-flask\\workdir\\pywps_process_uxjnrc6i",
                "allowed_values": [],
                "any_value": false,
                "mode": 1,
                "min_occurs": 1,
                "max_occurs": 1,
                "translations": null,
                "data": "Idan"
            }
        ]
    },
    "outputs": {
        "output": {
            "output": "",
            "mimetype": "",
            "encoding": "",
            "schema": "",
            "uom": ""
        }
    },
    "raw": true
}

Then I wanted to make a much more minimalistic request also valid as follows, so the following also works:

{
    "identifier": "say_hello",
    "outputs": "output",
    "inputs": {
        "name":"Idan"
    }
}

It is also possible to set the identifier in the endpoint url, so posting to /processes/say_hello is possible with the following input:

{
    "outputs": "output",
    "inputs": {
        "name":"Idan"
    }
}

Also possible to post to /processes/say_hello/output to indicate a raw output (for the output named output in this case), then such case the following is a valid input:

{
    "name":"Idan"
}

outputs can be a string (which implies raw output), or a list or a dict. Each input inside inputs can be a string (which implies literal input) or a dict.

Contribution Agreement

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

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

cehbrecht commented 3 years ago

@idanmiara Thanks for working on the ogcapi processes implementation :)

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

There is already work done for this in pygeoapi: https://github.com/geopython/pygeoapi/tree/master/pygeoapi/process

Did you have a look at it?

I would prefer if we could make make pywps a plugin of pygeoapi. If the rest ogcapi implementation is ready we might even skip the WPS xml one.

There is also work done for ogcapi as a proxy to an existing wps 1.0.0 implementation: https://github.com/crim-ca/weaver/tree/master/weaver/wps_restapi

We have also started prototyping on the OWSLib client side: https://github.com/geopython/OWSLib/issues/749

idanmiara commented 3 years ago

Hi MacPingu!

Sure, I'll reopen it on a dev branch, but maybe we better have a proper master branch to rebase on? Currently master is way behind, could someone take care of it please?

I didn't know about pygeoapi or how to make a plugin as you suggested.

I see that I'll need to do some work to get all the tests to pass again and maybe add some tests of the new api.

After the tests pass would someone want to help me take it from there ?

Idan

On Sun, 28 Mar 2021, 20:29 MacPingu, @.***> wrote:

@idanmiara https://github.com/idanmiara Thanks for working on the ogcapi processes implementation :)

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

There is already work done for this in pygeoapi: https://github.com/geopython/pygeoapi/tree/master/pygeoapi/process

Did you have a look at it?

I would prefer if could make make pywps a plugin of pygeoapi. If the rest ogcapi implementation is ready we might even skip the WPS xml one.

There is also work done for ogcapi as a proxy to an existing wps 1.0.0 implementation: https://github.com/crim-ca/weaver/tree/master/weaver/wps_restapi

We have also started prototyping on the OWSLib client side: geopython/OWSLib#749 https://github.com/geopython/OWSLib/issues/749

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/geopython/pywps/pull/588#issuecomment-808928721, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGJBBLONFARD5ODN236LVADTF5YR7ANCNFSM4Z52CB4Q .

idanmiara commented 3 years ago

I see that the pygeoapi is a separate implementation from pywps. If I had known it before I started this work I might have joint that effort instead...

How good is pygeoapi in comparison to pywps? Is it ready for production?

I already use a previous version of this implementation in production and it's working well enough for my needs. So I hope I can get this PR through fast with the hope it can help also for pygeoapi.

I'm not sure what you ment by saying: "If the rest ogcapi implementation is ready we might even skip the WPS xml one." Skip where?

Idan

On Sun, 28 Mar 2021, 21:02 Idan Miara, @.***> wrote:

Hi MacPingu!

Sure, I'll reopen it on a dev branch, but maybe we better have a proper master branch to rebase on? Currently master is way behind, could someone take care of it please?

I didn't know about pygeoapi or how to make a plugin as you suggested.

I see that I'll need to do some work to get all the tests to pass again and maybe add some tests of the new api.

After the tests pass would someone want to help me take it from there ?

Idan

On Sun, 28 Mar 2021, 20:29 MacPingu, @.***> wrote:

@idanmiara https://github.com/idanmiara Thanks for working on the ogcapi processes implementation :)

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

There is already work done for this in pygeoapi: https://github.com/geopython/pygeoapi/tree/master/pygeoapi/process

Did you have a look at it?

I would prefer if could make make pywps a plugin of pygeoapi. If the rest ogcapi implementation is ready we might even skip the WPS xml one.

There is also work done for ogcapi as a proxy to an existing wps 1.0.0 implementation: https://github.com/crim-ca/weaver/tree/master/weaver/wps_restapi

We have also started prototyping on the OWSLib client side: geopython/OWSLib#749 https://github.com/geopython/OWSLib/issues/749

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/geopython/pywps/pull/588#issuecomment-808928721, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGJBBLONFARD5ODN236LVADTF5YR7ANCNFSM4Z52CB4Q .

cehbrecht commented 3 years ago

Sure, I'll reopen it on a dev branch, but maybe we better have a proper master branch to rebase on? Currently master is way behind, could someone take care of it please?

The master will be enabled again ... see #590

The current pywps-4.4 version will become the master version.

cehbrecht commented 3 years ago

How good is pygeoapi in comparison to pywps? Is it ready for production?

pygeoapi is implementing the ogcapi protocols on the server side. It is meant to handle the service protocol but the real work can be handled in plugins. It has only a "dummy" implementation for processes.

I thought the 5.x version of pywps could be pygeoapi plugin. We want to keep the process integration, the output handling, different queuing systems (multiprocessing, slurm, ...). But the protocol is handled by pygeoapi.

The 5.x version of pywps would then be a ogcapi only release. The WPS 1.0.0 stays in 4.x.

idanmiara commented 3 years ago

Thanks for the explanation. I suggest to merge this PR once its ready to v 4.6 then continue to a plugin 5.0 if it makes sence.

On Mon, 29 Mar 2021, 13:06 MacPingu, @.***> wrote:

How good is pygeoapi in comparison to pywps? Is it ready for production?

pygeoapi is implementing the ogcapi protocols on the server side. It is meant to handle the service protocol but the real work can be handled in plugins. It has only a "dummy" implementation for processes.

I thought the 5.x version of pywps could be pygeoapi plugin. We want to keep the process integration, the output handling, different queuing systems (multiprocessing, slurm, ...). But the protocol is handled by pygeoapi.

The 5.x version of pywps would then be a ogcapi only release. The WPS 1.0.0 stays in 4.x.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/geopython/pywps/pull/588#issuecomment-809253095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGJBBLJF2OSVPDRB5TYECVDTGBNLLANCNFSM4Z52CB4Q .

tomkralidis commented 3 years ago

pygeoapi is implementing the ogcapi protocols on the server side. It is meant to handle the service protocol but the real work can be handled in plugins. It has only a "dummy" implementation for processes.

The dummy implementation is default support for job management. processes themselves are implemented as plugins (see https://docs.pygeoapi.io/en/latest/plugins.html#featured-plugins for examples).

Having said this, it would be great to also have PyWPS as a processing backend concept for pygeoapi (happy to discuss).

idanmiara commented 3 years ago

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

@cehbrecht Please create dev-ogcapi from 4.4/master, I think cannot create it myself.

cehbrecht commented 3 years ago

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

@cehbrecht Please create dev-ogcapi from 4.4/master, I think cannot create it myself.

Can I create it when master is ready? Hopefully over easter. Otherwise I will make it from the pywps.4.4 branch.

idanmiara commented 3 years ago

I would prefer to merge this work to a dev branch. Name dev-ogcapi?

@cehbrecht Please create dev-ogcapi from 4.4/master, I think cannot create it myself.

Can I create it when master is ready? Hopefully over easter. Otherwise I will make it from the pywps.4.4 branch.

Sure. Hopefully it will be ready to be merged by that time.

ldesousa commented 3 years ago

Hi all,

First thank you @idanmiara from contributing to PyWPS, much appreciated.

I am sick at the moment, I won't be able to do an in-depth review at least in the coming days. PyWPS and pygeoapi are not really alternatives at the moment, so I would favour merging as soon as the tests pass. I see there are relevant changes to the core, so not a bad idea to create a separate branch.

I like the idea of PyWPS 5.0 becoming a plug-in to pygeoapi. On the other hand I would not like to drop support to WPS 2.0 already. I think the PSC needs to give a bit of thought to this.

Cheers.

huard commented 3 years ago

I think it's great that PyWPS can implement the OGC API during the development of the Processes specifications. One thing I'm a bit worried about is that the specs are still changing, so more work will probably be required to keep PyWPS in sync with the specs until they are finalized.

I only took a very superficial look to the code but didn't see any unit tests. Now since the specs are moving, I'm not sure it's such a good idea to invest time in unit tests, but this is a discussion worth having I think.

idanmiara commented 3 years ago

I've rebased it on https://github.com/geopython/pywps/pull/594, as I'm testing it on Windows. All the tests pass now :)

idanmiara commented 3 years ago

Hi all,

First thank you @idanmiara from contributing to PyWPS, much appreciated.

I am sick at the moment, I won't be able to do an in-depth review at least in the coming days. PyWPS and pygeoapi are not really alternatives at the moment, so I would favour merging as soon as the tests pass. I see there are relevant changes to the core, so not a bad idea to create a separate branch.

I like the idea of PyWPS 5.0 becoming a plug-in to pygeoapi. On the other hand I would not like to drop support to WPS 2.0 already. I think the PSC needs to give a bit of thought to this.

Cheers.

Thanks, get well soon 👍

idanmiara commented 3 years ago

I think it's great that PyWPS can implement the OGC API during the development of the Processes specifications. One thing I'm a bit worried about is that the specs are still changing, so more work will probably be required to keep PyWPS in sync with the specs until they are finalized.

I've tried to implement the core features of the spec. I didn't go over the top to implement the things I didn't use, but we need to start somewhere...

I only took a very superficial look to the code but didn't see any unit tests. Now since the specs are moving, I'm not sure it's such a good idea to invest time in unit tests, but this is a discussion worth having I think.

Are there some tests that we can use from pygeoapi or other source?

idanmiara commented 3 years ago

Please review/approve https://github.com/geopython/pywps/pull/594 before you review this PR as I've rebased on it.

huard commented 3 years ago

Don't get me wrong, I'm super glad you are working on this, I'm only suggesting we're advertising this functionality as "experimental - not for production".

For testing, there is a WPSClient in https://github.com/geopython/pywps/blob/20e1e254a3f7914e555fa89f363d1f6eb5f3895c/pywps/tests.py#L87

I suppose implementing a post_json method would allow you to test the REST interface, but its more a guess than anything.

Related: we started work on a client a couple of weeks ago: https://github.com/geopython/OWSLib/pull/750 As you'll, there are considerable differences across implementations, but I think it would make sense for the OWSLib client to stay in sync with the WPS implementation.

coveralls commented 3 years ago

Coverage remained the same at 0.0% when pulling 54c1b0932f71225d2c8d6b84a9cb5e665c88f938 on talos-gis:pywps-4.4-rest into 711219792be8b3d6a175a81152282dc5046d412b on geopython:main.

idanmiara commented 3 years ago

Hi, Can anyone check why the tests fail? As far as I can see the fail also without this PR.

cehbrecht commented 3 years ago

Hi, Can anyone check why the tests fail? As far as I can see the fail also without this PR.

looks like opendap test server is not available: http://test.opendap.org/opendap/netcdf/examples/sresa1b_ncar_ccsm3_0_run1_200001.nc

idanmiara commented 3 years ago

I can add add a skip in case it's unavailable. Is the other test failure for the same reason?

On Tue, 27 Apr 2021, 20:09 MacPingu, @.***> wrote:

Hi, Can anyone check why the tests fail? As far as I can see the fail also without this PR.

looks like opendap test server is not available:

http://test.opendap.org/opendap/netcdf/examples/sresa1b_ncar_ccsm3_0_run1_200001.nc

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/geopython/pywps/pull/588#issuecomment-827769257, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGJBBLI455DAJ2UYDJTPBI3TK3VVPANCNFSM4Z52CB4Q .

ldesousa commented 3 years ago

Looks like that is the only failure at the moment. Try to skip that test, or replace it with a different NetCDF source.

idanmiara commented 3 years ago

Now all the tests pass and I've added a couple for the REST API. How do you advise to proceed to get it merged ?

idanmiara commented 3 years ago

I think this is a valuable addition to PyWPS. This PR could use a few more docstrings.

In the future, I would like to see documentation and more tests, but I suggest we open an issue and do this later when the API is finalized.

@huard Thanks very much for your review! I think I've addressed the issues you raised.

cehbrecht commented 3 years ago

@idanmiara can you rebase this PR to the new main branch?

idanmiara commented 3 years ago

@idanmiara can you rebase this PR to the new main branch?

Done.

cehbrecht commented 3 years ago

I have checked this PR with emu ... and it is working (wps 1.0.0).

cehbrecht commented 3 years ago

@ldesousa @huard ok to merge into main branch as experimental implementation of ogcapi-processes?

cehbrecht commented 3 years ago

I have seen today that there is a ogcapi-processes workshop this week: https://github.com/opengeospatial/ogcapi-processes/blob/master/workshops/2021_06_Workshop.adoc

ldesousa commented 3 years ago

@ldesousa @huard ok to merge into main branch as experimental implementation of ogcapi-processes?

Yes. I think it might need a few more tests, but we should integrate @idanmiara's contribution now and later improve if necessary.

idanmiara commented 3 years ago

Thanks guys! I didn't want to invest too much energy into more testing and docs before I get your general approval.

cehbrecht commented 3 years ago

@idanmiara merged. Thanks for your nice work and getting started on the rest-api :)

idanmiara commented 3 years ago

Thanks @cehbrecht ! Can you make a first 4.5 release to encourage more people to check it out sooner?

cehbrecht commented 3 years ago

@idanmiara Would you like to add an example for the current rest-api to the docs? Not too much ... maybe an example with a "hello" or "sleep" process ... something we can point to. I can prepare the 4.5 release then.

idanmiara commented 3 years ago

@idanmiara Would you like to add an example for the current rest-api to the docs? Not too much ... maybe an example with a "hello" or "sleep" process ... something we can point to. I can prepare the 4.5 release then.

Sure

idanmiara commented 3 years ago

@idanmiara Would you like to add an example for the current rest-api to the docs? Not too much ... maybe an example with a "hello" or "sleep" process ... something we can point to. I can prepare the 4.5 release then.

https://github.com/geopython/pywps/pull/612

idanmiara commented 3 years ago

@cehbrecht please let me know if you need further help with this release :)

geopython / pywps

OGC API - Processes / REST API implementation #588

Overview

Example JSON requests:

Contribution Agreement