Process unification #310 #348

m-mohr · 2020-12-07T15:00:05Z

This is a first draft for #310 that tries to unify /processes and /process_graphs.

Sharing might be a separate PR.

soxofaan

(initial review)

openapi.yaml

soxofaan · 2020-12-08T12:02:18Z

openapi.yaml

+        For ease of use, it is NOT RECOMMENDED to use long randomly generated
+        identifiers. More readable user identifiers like `john_doe` support a
+        better user-experience as the user identifier is used in URIs for shared
+        processes, e.g. `https://example.org/api/v1.0/processes/@john_doe/my_ndvi`.


I agree that john_doe is nicer, but this recommendation does not play well with what OIDC gives us. For example google returns user ids like us-a44d63b6-e090-6059-81d7-cbe7afeff6ce and microsoft: nIrHDS4rhk4ri738TRhtLHXdoUQ6OxZo9Ob0AS3vTig

there is a piece of the puzzle missing here

If there's no user name / id to derive such a slug from, what to use instead?

indeed, I'm not even sure the returned userid is stable, e.g. if a user registers a new client id to work with, maybe an OIDC providers could bump/rehash the userid?

In general, I would expect that a back-end assigns (or let the user choose) a separate user-id in addition to the external user-id. That is how I've seen it implemented as in most all cases you need to store additional user data anyway.

openapi.yaml

soxofaan · 2020-12-08T12:07:51Z

openapi.yaml

      description: |-
-        Lists all user-defined processes (process graphs) of the
+        Redirects to all user-defined processes of the


Also add some kind of deprecation notice to the description of these endpoints?

At least it should probably describe what to use/do instead. But I'm still not sure whether to use this redirect behavior (breaking) or leave /process_graphs as it is (basically as an alias, but non-breaking).

how do you mean that the redirect is breaking? That (some) clients don't properly handle a redirect on POST/PUT/PATCH/DELETE?

Yes, I (have to) assume that not all clients would handle it properly. But if we test JS, R and Python and all work flawlessly, it's probably okay to call it non-breaking ;-)

soxofaan · 2020-12-08T12:09:37Z

openapi.yaml

+        The namespace `backend` is an alias for predefined processes.
+
+        Back-end implementations MAY implement other namespaces that don't
+        conflict with any of the namespaces mentioned above.


see discussion at #310 (comment)

I think the namespace "format" should be more flexible/generic than: @ is for per-user namespaces

And I disagree in #310 (comment) ;-)

In the meantime I'm fine with more flexibility, but on the other hand we should likely restrict the allowed characters:
#478

m-mohr · 2021-05-07T09:50:37Z

Seems not stable enough for 1.1, so moving to 1.2. Would be good to have an implementation first, too.

soxofaan · 2021-05-12T11:40:53Z

openapi.yaml

@@ -1617,6 +1632,187 @@ paths:
          $ref: '#/components/responses/client_error_auth'
        5XX:
          $ref: '#/components/responses/server_error'
+  '/processes/{namespace}':
+    get:
+      summary: List all user-defined processes in a namespace


Documentation of /processes/{namespace} and /processes/{namespace}/{process_id} talk about "user-defined processes in a "namespace", but listing "predefined" processes should also work, right?

e.g. GET /processes/backend returns the same as GET /processes
and GET /process/backend/filter_temporal would return the metadata of filter_temporal process?

I think the thinking was that there would be duplication as you've pointed out. You usually get all details from /processes for pre-defined processes so that this "extension" is mostly for user-defined processes (and was mostly copied from /process_graphs). So we can discuss whether we should remove the "user-defined" here. The backend namespace is somewhat different though as it is read-only...

I don't think that de duplication between GET /processes/backend and GET /processes is that much of an issue, especially because "backend" can be considered to be a default namespace. So yes, I would argue that the "user-defined" can be dropped here.

Also, a back-end is also free to define custom namespaces, and these could also contain pre-defined, non-user-defined, "read only", possibly proprietary processes.

~~Yes, I'm fine with that. We can add additional wording that clarifies any specifics for user-defined processes.~~ Edit: It's not that simple, see below...

Hmm, looking into the OpenAPI file made me remember that user-defined processes and pre-defined processes have a different schema. So indeed the endpoints were only meant to support user-defined processes so far.

User-defined processes for example require a process_graph, but that's not possible for predefined processes.
On the other hand, pre-defined processes require e.g. parameters and return values while this is optional for user-defined processes.

we need to split up into two separate endpoints to make sure the schemas can be applied correctly
Or we don't allow exposing this endpoint as it's already exposed via /processes.

Both options are equally bad for us, VITO, as we are already using a non-"backend" namespace containing predefined proprietary processes (without a "process_graph"), which would be invalid according to both of these options.

Hmm... maybe we can add a discriminator to GET /processes/{namespace}. If it contains type: user: true (or something similar) in the response it applies the user schema to the processes array, otherwise the predefined schema. That would also help clients to know whether they can do non-GET requests on that namespace.

I think part of the current problem with "predefined" vs "user-defined", is that we are lumping together a couple of process concepts, for example:

predefined user defined

live in default namespace live in "user" namespace

implemented "natively" by backend implemented through openEO process graph

has no "process_graph" field in metadata has a "process_graph" field in metadata

public (by default?) private (by default?)

no public API to add/update/remove created and managed though openEO endpoints

parameters and return values must be declared parameters and return values are optional

By sticking to this binary division, each with own "schema", we probably make it hard for ourselves to create a clean API in the long term.
For example, in VITO backend we already have processes that mix properties from both columns, e.g. private, natively implemented processes that live in a namespace that is neither default or per-user. Another example is defining predefined processes through a "process_graph", instead of "natively" (e.g. "ndvi" or "evi").

Please remember that we are just moving things around here. This is to move the endpoints to /processes/... for unification and to prepare for v2, but we have to stay compliant with the API v1.x line for now. We can't change a lot wrt the schemas for example as that usually is a breaking change. So until we go for API v2 we may need to live with some compromises.

sure, I understand, I was just reflecting on the tension between the constraints of the v1.x API and what we are doing in VITO backend (or want to do) with custom namespaces

ref: Open-EO/openeo-api#348

# Conflicts: # openapi.yaml

m-mohr · 2021-08-19T11:29:57Z

Some additional thoughts on namespaces:

The namespaces list in /processes just lists the identifiers. For some use-cases it could be useful that this is expansible, e.g. with a title, a description, a boolean loadByDefault (I guess mostly for the Web Editor?) or a type (e.g. predefined or user-defined)
Requesting the processes for a namespace could also include the additional metadata for the namespace as mentioned above (excluding loadByDefault I guess).

Lastly, I'm thinking to not merge this into the "core" API, but instead, make this a separate extension. Then this would be a pure addition and the /process_graphs would stay as they are. Final unification would then be done in v2.

soxofaan · 2021-08-19T12:46:12Z

Final unification would then be done in v2.

I'm afraid I have to agree 😄 .
It's probably not feasible to achieve unification in a clean, non-breaking way under v1

m-mohr · 2021-12-20T10:56:04Z

openapi.yaml


        If multiple processes with the same identifier exist, Clients SHOULD
        inform the user that it's recommended to select a namespace.
+    process_namespace:
+      type: string
+      pattern: ^@?[\w\-\.~]+$


For VITO we'd need to add a double colon here (i.e. for the u:asd replacement for @GreatEmerald )

m-mohr added process discovery and profile discovery process graph management feedback required breaking Breaking changes, requires a major-version (2.0.0 for example) labels Dec 7, 2020

m-mohr added this to the future milestone Dec 7, 2020

m-mohr requested review from soxofaan and jdries December 7, 2020 15:00

m-mohr self-assigned this Dec 7, 2020

m-mohr force-pushed the process-unification branch 4 times, most recently from 271042c to 9b4e914 Compare December 7, 2020 16:10

First draft to unify /processes and /process_graphs #310

541e9f9

m-mohr force-pushed the process-unification branch from 9b4e914 to 541e9f9 Compare December 7, 2020 17:32

soxofaan mentioned this pull request Dec 8, 2020

Unification of /processes and /process_graphs #310

Open

soxofaan reviewed Dec 8, 2020

View reviewed changes

m-mohr mentioned this pull request Mar 5, 2021

Issue #182 add namespace support to DataCube.process and related Open-EO/openeo-python-client#183

Merged

m-mohr mentioned this pull request Apr 8, 2021

2.0.0 clean-up #374

Open

6 tasks

m-mohr linked an issue Apr 8, 2021 that may be closed by this pull request

Unification of /processes and /process_graphs #310

Open

m-mohr mentioned this pull request Apr 13, 2021

Requesting processes very slow Open-EO/openeo-python-driver#62

Closed

m-mohr force-pushed the draft branch from 94f3496 to fa2ee5f Compare April 14, 2021 15:39

m-mohr modified the milestones: future, 1.1.0, 1.2.0 May 5, 2021

m-mohr mentioned this pull request May 12, 2021

metadata of a single (predefined) process #392

Closed

m-mohr linked an issue May 12, 2021 that may be closed by this pull request

metadata of a single (predefined) process #392

Closed

soxofaan reviewed May 12, 2021

View reviewed changes

soxofaan added a commit to Open-EO/openeo-python-driver that referenced this pull request May 12, 2021

add initial support for GET /processes/{namespace}/{process_id}

4a04fd5

ref: Open-EO/openeo-api#348

m-mohr added a commit to Open-EO/openeo-js-client that referenced this pull request Aug 5, 2021

Experimental process namespace support Open-EO/openeo-api#348

aac1a11

m-mohr added a commit to Open-EO/openeo-js-client that referenced this pull request Aug 5, 2021

Experimental process namespace support Open-EO/openeo-api#348

1c1beb3

m-mohr mentioned this pull request Aug 5, 2021

Experimental process namespace support Open-EO/openeo-js-client#52

Merged

m-mohr added a commit to Open-EO/openeo-js-client that referenced this pull request Aug 5, 2021

Experimental process namespace support Open-EO/openeo-api#348

b84b2c2

m-mohr added a commit to Open-EO/openeo-js-client that referenced this pull request Aug 5, 2021

Experimental process namespace support Open-EO/openeo-api#348

3e3b514

m-mohr added 4 commits August 17, 2021 14:41

Merge remote-tracking branch 'origin/draft' into process-unification

aef48f0

# Conflicts: # openapi.yaml

Add namespace pattern, fix authentication details

daa18eb

pre-defined => predefined, backend => back-end

e5ac184

Merge remote-tracking branch 'origin/draft' into process-unification

af17c49

# Conflicts: # openapi.yaml

m-mohr force-pushed the draft branch from e5ac184 to 66b7483 Compare August 17, 2021 13:06

m-mohr added 2 commits August 17, 2021 15:07

Merge remote-tracking branch 'origin/draft' into process-unification

3720c94

# Conflicts: # openapi.yaml

Rephrased canonical link description #408

738e327

m-mohr modified the milestones: 1.2.0, 1.3.0 Nov 29, 2021

m-mohr removed their assignment Dec 1, 2021

m-mohr commented Dec 20, 2021

View reviewed changes

m-mohr mentioned this pull request Dec 20, 2021

fix: added a function to normalize the namespace in case of UDP Open-EO/openeo-js-client#57

Merged

m-mohr mentioned this pull request May 24, 2022

UDP namespace with @ not allowed/supported? Open-EO/openeo-web-editor#260

Closed

m-mohr mentioned this pull request Dec 23, 2022

Allowed characters: User IDs and namespaces #478

Open

Base automatically changed from draft to master May 25, 2023 08:23

m-mohr changed the base branch from master to draft June 1, 2023 11:09

m-mohr force-pushed the draft branch from 05bc0ed to cecc3f4 Compare October 11, 2023 16:17

This was referenced Jun 13, 2024

Allow URLs as process namespace #515

Closed

Normalize namespace doesn't work for EGI Checkin users Open-EO/openeo-js-client#60

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Process unification #310 #348

Process unification #310 #348

m-mohr commented Dec 7, 2020

soxofaan left a comment

soxofaan Dec 8, 2020

m-mohr Dec 8, 2020 •

edited

Loading

soxofaan Dec 8, 2020

m-mohr Dec 8, 2020 •

edited

Loading

soxofaan Dec 8, 2020

m-mohr Dec 8, 2020

soxofaan Dec 8, 2020

m-mohr Dec 8, 2020 •

edited

Loading

soxofaan Dec 8, 2020

m-mohr Dec 8, 2020 •

edited

Loading

m-mohr Dec 23, 2022

m-mohr commented May 7, 2021

soxofaan May 12, 2021

m-mohr May 12, 2021

soxofaan May 12, 2021

m-mohr May 12, 2021 •

edited

Loading

m-mohr May 12, 2021 •

edited

Loading

soxofaan May 12, 2021

m-mohr May 14, 2021 •

edited

Loading

soxofaan May 17, 2021

m-mohr May 17, 2021 •

edited

Loading

soxofaan May 17, 2021

m-mohr commented Aug 19, 2021 •

edited

Loading

soxofaan commented Aug 19, 2021

m-mohr Dec 20, 2021 •

edited

Loading

predefined	user defined
live in default namespace	live in "user" namespace
implemented "natively" by backend	implemented through openEO process graph
has no "process_graph" field in metadata	has a "process_graph" field in metadata
public (by default?)	private (by default?)
no public API to add/update/remove	created and managed though openEO endpoints
parameters and return values must be declared	parameters and return values are optional

Process unification #310 #348

Are you sure you want to change the base?

Process unification #310 #348

Conversation

m-mohr commented Dec 7, 2020

soxofaan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr commented May 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr May 12, 2021 • edited Loading

Choose a reason for hiding this comment

m-mohr May 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr May 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr May 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-mohr commented Aug 19, 2021 • edited Loading

soxofaan commented Aug 19, 2021

m-mohr Dec 20, 2021 • edited Loading

Choose a reason for hiding this comment

m-mohr Dec 8, 2020 •

edited

Loading

m-mohr Dec 8, 2020 •

edited

Loading

m-mohr Dec 8, 2020 •

edited

Loading

m-mohr Dec 8, 2020 •

edited

Loading

m-mohr May 12, 2021 •

edited

Loading

m-mohr May 12, 2021 •

edited

Loading

m-mohr May 14, 2021 •

edited

Loading

m-mohr May 17, 2021 •

edited

Loading

m-mohr commented Aug 19, 2021 •

edited

Loading

m-mohr Dec 20, 2021 •

edited

Loading