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

"Run now" mode for web services #277

Closed
m-mohr opened this issue Sep 9, 2022 · 23 comments
Closed

"Run now" mode for web services #277

m-mohr opened this issue Sep 9, 2022 · 23 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@m-mohr
Copy link
Member

m-mohr commented Sep 9, 2022

Currently, the web services work as follows:

  • Create a user-defined process in the Editor
  • Click Create in the Web Service tab
  • Input title, web service type, ...
  • Click submit button
  • Click show on map
  • Work with web service
  • Close map/tab
  • Delete web service (if you want)

It would be more user-friendly if there would also be a "quick run" mode, similar to synchronous processing:

  • Create a user-defined process in the Editor
  • Click "Show on map" or "Preview on Map" in the Web Service tab
  • Create web service automatically and show it on the map - users still may need to choose the web service type if multiple web service types are supported (both by the map viewer and the back-end)
    • Pre-fill fields, e.g. title could be the date and time
    • Always use default options for parameters etc
  • Work with web service
  • Once the map/tab is closed or the user logs out/disconnects, delete the service - be aware that it won't be deleted if the browser is getting closed while the map is still open

cc @dthiex (funded through a openEO Platform SAP)

@m-mohr m-mohr added the enhancement New feature or request label Sep 9, 2022
@m-mohr m-mohr self-assigned this Sep 9, 2022
@m-mohr m-mohr changed the title "Run" mode for web services "Run now" mode for web services Sep 9, 2022
@Ardweaden
Copy link

What would you think about the following approach:

  1. We would use a single secondary service type and user wouldn't have an option to choose
  2. We would add on-demand-viewer to the configurations options of the service
  3. When user would click "Show on map", the editor would make a request to publish a new service with on-demand-viewer: true in the configurations field. It would send the process graph as-is.
  4. We would then treat services with this configuration differently:
    • service would not be listed at /services
    • service would have expiration time, upon which it would get deleted (e.g. 1 hour)
    • internally, we would parametrise the spatial extent to use the parameters etc. or any other required changes to the process graph. Additionally, we may add some auto-scaling for nicer visualisation

We have to support this in Jupyter Notebook too, so I'd prefer not to write the code for this in multiple places.

@m-mohr
Copy link
Member Author

m-mohr commented Sep 14, 2022

Hmm, honestly, I don't like it (but I might be biased):

  • It's not specified in the API. At the moment, I'd rather try to make it work with the existing API specification, otherwise, we'd have another round of spec work first (and that's not in the SAP proposal so there's no time assigned for it). The implementation in the Web Editor would also be Platform-specific, which I'd like to avoid.
  • Also, the proposal is also not really RESTish in that you post something to /services, but then it's missing in the list of services. That feels counter-intuitive.

@Ardweaden
Copy link

How about the part of modifying the process graph within the service to match its needs (parametrised spatial extent) as opposed to doing it on the client side?

@m-mohr
Copy link
Member Author

m-mohr commented Sep 14, 2022

The API doesn't prescribe this and it is very much up to the type of service how it is handled (e.g. via process parameters or implicitly via other information from the context).

For example, the XYZ implementation in the GEE back-end does it implicitly, so the extents given in load_collection just restrict the overall extent that it is allowed to process on. The extent for a specific tile is then derived from the X, Y and Z given in the URL. I assume you can follow a similar approach?

@Ardweaden
Copy link

For example, the XYZ implementation in the GEE back-end does it implicitly, so the extents given in load_collection just restrict the overall extent that it is allowed to process on. The extent for a specific tile is then derived from the X, Y and Z given in the URL. I assume you can follow a similar approach?

No, our XYZ secondary service returns the results of the process graph as it is defined. That is, if a "static" process graph (with fixed spatial extent) is defined, every tile will look the same. User can however use parameters that our XYZ service defines (coordinates of the tile bounding box) wherever they want in the process graph to get dynamic results - logically they would use it in the load_collection's spatial extent.

@m-mohr
Copy link
Member Author

m-mohr commented Sep 14, 2022

Why though? That seems more complex than it would need to be...

@Ardweaden
Copy link

It is not very complex to implement, the available parameters are simply injected into the process graph when a tile is requested, and if none are used, it just goes through unchanged. Reading the standard this is how it seemed like the implementation was intended to be done.

@m-mohr
Copy link
Member Author

m-mohr commented Sep 14, 2022

I know which part of the API spec you refer to, but that's only an example. So from the API perspective, you can also implement the implicit behavior, which is the easier approach for the user as he does not need to apply all the parameters in the spatial_extent. You also just run into fewer issues where used did not use the parameters. At least in XYZ the spatial extent for each tile seems well-defined.

@Ardweaden
Copy link

OK, we can change it so that if no parameters are used explicitly, we overwrite the spatial extent in the load collection and that should enable this. So on our side this would then be the only thing we should change on our backend.
I do still think, however, that the solution with parameters is a bit less "hand-wavy" overall.

@Ardweaden
Copy link

This change was now deployed to https://openeo.sentinel-hub.com/production/, which is also included in the testing aggregator.
That is, if not parameters are used, spatial_extent is overwritten with service parameters.

I assume you will implement it in the editor?

m-mohr added a commit that referenced this issue Oct 10, 2022
m-mohr added a commit that referenced this issue Oct 10, 2022
@m-mohr m-mohr added this to the v0.12 milestone Oct 10, 2022
@m-mohr
Copy link
Member Author

m-mohr commented Oct 10, 2022

v0.11.3 has been deployed and supports this behavior.

Unfortunately, I could only test it with GEE as my process returns a "'NoneType' object is not subscriptable" error from the Sinergise back-end. Do you have a process for testing purposes at hand?

Process:

{
  "process_graph": {
    "load1": {
      "process_id": "load_collection",
      "arguments": {
        "id": "VEGETATION_INDICES",
        "spatial_extent": null,
        "temporal_extent": [
          "2016-10-01T09:20:22Z",
          "2016-10-10T09:20:22Z"
        ],
        "bands": [
          "NDVI"
        ]
      }
    },
    "save2": {
      "process_id": "save_result",
      "arguments": {
        "data": {
          "from_node": "reduce3"
        },
        "format": "PNG",
        "options": {
          "datatype": "byte"
        }
      },
      "result": true
    },
    "reduce3": {
      "process_id": "reduce_dimension",
      "arguments": {
        "data": {
          "from_node": "load1"
        },
        "dimension": "t",
        "reducer": {
          "process_graph": {
            "mean1": {
              "process_id": "mean",
              "arguments": {
                "data": {
                  "from_parameter": "data"
                }
              },
              "result": true
            }
          }
        }
      }
    }
  },
  "parameters": []
}

@m-mohr m-mohr closed this as completed Oct 10, 2022
@dthiex
Copy link

dthiex commented Oct 10, 2022

This one always used to work (you will probably need to still adjust the spatial extent)

{
    "parameters": [],
    "process_graph": {
      "1": {
        "arguments": {
          "bands": [
            "B05",
            "B04",
            "B03"
          ],
          "id": "sentinel-2-l2a",
          "spatial_extent": {
            "east": {
              "from_parameter": "spatial_extent_east"
            },
            "north": {
              "from_parameter": "spatial_extent_north"
            },
            "south": {
              "from_parameter": "spatial_extent_south"
            },
            "west": {
              "from_parameter": "spatial_extent_west"
            }
          },
          "temporal_extent": [
            "2022-03-20T00:00:00Z",
            "2022-03-31T00:00:00Z"
          ]
        },
        "process_id": "load_collection"
      },
      "2": {
        "arguments": {
          "data": {
            "from_node": "3"
          },
          "format": "PNG",
          "options": {
            "datatype": "byte"
          }
        },
        "process_id": "save_result",
        "result": true
      },
      "3": {
        "arguments": {
          "data": {
            "from_node": "1"
          },
          "process": {
            "process_graph": {
              "1": {
                "arguments": {
                  "inputMax": 1,
                  "inputMin": 0,
                  "outputMax": 255,
                  "x": {
                    "from_parameter": "x"
                  }
                },
                "process_id": "linear_scale_range",
                "result": true
              }
            }
          }
        },
        "process_id": "apply"
      }
    }
  }

@m-mohr
Copy link
Member Author

m-mohr commented Oct 10, 2022

Thanks, this gives me a result for some tiles at least. Anyway, it is implemented and deployed. Please let me know if you need any further changes.

@dthiex
Copy link

dthiex commented Oct 17, 2022

@m-mohr I wanted to start writing some documentation on this (part of the deliverable) but can't find the functionality. Can you maybe point me to where you implemented this?

@dthiex
Copy link

dthiex commented Oct 17, 2022

@m-mohr I wanted to start writing some documentation on this (part of the deliverable) but can't find the functionality. Can you maybe point me to where you implemented this?

Nevermind, I went through the issue description and actually found it:

grafik

@Ardweaden
Copy link

Shouldn't this functionality be available in the Data Processing tab?

@dthiex
Copy link

dthiex commented Oct 17, 2022

Plus: What would we need to do to also have this in the Platform editor? (I think this is where Patrick expects it).

Obviously we would need to get our backend into the production aggregator. Further, I don't see web service functionality available there. What was the reason this was left out? (If this feature would be within data processing this wouldn't be a problem).

@m-mohr
Copy link
Member Author

m-mohr commented Oct 17, 2022

Shouldn't this functionality be available in the Data Processing tab?

Hmm, not sure. Originally the "Data Processing" tab was "Batch Jobs" and got just renamed because we added the "Run now" for synchronous due to the lack of a better place. I see reasons for both... maybe add it to both? Not sure...

Plus: What would we need to do to also have this in the Platform editor? (I think this is where Patrick expects it).

I was waiting for a confirmation from you two that this works for you as expected in editor.openeo.org. If you can confirm that the work is finished, I can deploy it to the Platform Editor, too. But: The aggregator needs web service functionality, so additions to GET / in endpoints and the new endpoints GET /service_types and /services/...

Further, I don't see web service functionality available there. What was the reason this was left out?

The aggregator need to be updated. The Web Editor works adaptive and only shows what the back-end supports. So the aggregator needs to be updated with the corresponding functionality.

(If this feature would be within data processing this wouldn't be a problem).

No, it would still not show up until the aggregator adds the web service functionality.

@Ardweaden
Copy link

Hmm, not sure. Originally the "Data Processing" tab was "Batch Jobs" and got just renamed because we added the "Run now" for synchronous due to the lack of a better place. I see reasons for both... maybe add it to both? Not sure...

I actually think the most logical location would be among the process graph tools, next to "validate on server-side". Given this feature is all about helping to test out the process graph while creating it.

@m-mohr
Copy link
Member Author

m-mohr commented Oct 17, 2022

We had the "Run now" (as play icon) there in the beginning but many people did not realize that it's there and what it is for, so I moved it to a more prominent place where you can also add text.
Also, I don't fully agree this is "all about helping to test", for me it is even more. It's on-the-fly processing on openEO Platform like e.g. for exploration and visualization in GEE and as such deserves a more prominent place.

I'll think about how to best handle the "Data Processing" thing with the buttons... I was wondering whether it makes sense to merge Batch Jobs and Web Services....

@m-mohr
Copy link
Member Author

m-mohr commented Oct 17, 2022

Please see #283

@Ardweaden
Copy link

@m-mohr
What do you think about adding "expiration date" or something similar to openEO spec for services? As in the idea that we could have services automatically deleted after some time, which would be very useful for on-demand-viewer, especially in case of the Jupyter Notebook, where there is no user action that would trigger the deletion.

@m-mohr
Copy link
Member Author

m-mohr commented Oct 28, 2022

Moved to Open-EO/openeo-api#465 because it is mostly unrelated to the Web Editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants