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

/preview does not really align with use cases of sync api calls #162

Closed
jdries opened this issue Jan 4, 2019 · 5 comments
Closed

/preview does not really align with use cases of sync api calls #162

jdries opened this issue Jan 4, 2019 · 5 comments
Milestone

Comments

@jdries
Copy link

jdries commented Jan 4, 2019

In 0.3.x, synchronous API calls now go through '/preview' (https://open-eo.github.io/openeo-api/v/0.3.1/apireference/#tag/Job-Management/paths/~1preview/post)
The use case of synchronous api calls goes beyond previewing stuff:

  • retrieving time series in a synchronous manner
  • simply downloading an image, for analysis
  • any process that generates a meaningful result without requiring asynchronous handling

What was wrong with 'execute'?

@jdries jdries added the job management incl. /result label Jan 4, 2019
@m-mohr
Copy link
Member

m-mohr commented Feb 5, 2019

execute sounds like it could be used for heavy processing, but as it is synchronous it will be stopped once the browser/terminal is closed, may time out or return results in a size that the client can't handle. It is also not quire clear from the names what the difference between execute and jobs is. Therefore I found execute misleading, still preview might not be the best name either. In addition, I tried to be consistent with the endpoint names and follow the "REST-style" to use nouns instead of verbs.

Personally, I'd not promote preview too much and let batch jobs and web services handle the use cases as it is quite limited in some regards.

If I remember correctly, /execute was introduced during the Münster sprint to only have an easier way of implementing some processing for a proof of concept without requiring to implement the asynchronous handling. I thought we would remove it later anyway (and maybe replace it with batch job + some convenience shortcuts in the clients).

If we want to keep it in the long run and there's any name that fits better and follows the current "style", I'm happy to adopt it. I didn't found a better one yet and I would not like to introduce execute again.

@jdries
Copy link
Author

jdries commented Feb 5, 2019

Thanks for the explanation, it clarifies the reasoning. As already stated, I have a bit of a different view on relevance of the different calls:

  • synchronous calls (preview/execute) are basically the way of the future. Both GEE, SentinelHub, and some services at VITO already show that meaningful information can be returned within the timing constraints of a synchronous call.
  • asynchronous calls (batch jobs) are also needed, but especially for backends that support UDF's, where the performance of the process graph is out of the control of the backend. I don't see it as a must-have feature for every backend implementation.
  • web services for viewing are also very relevant, but there I've been thinking about supporting it also in a stateless manner, where the process graph is sent along with each request in a custom parameter. This way of working is also supported by SentinelHub. It allows a backend to work without requiring user management or state.

Now all of the above is just context to explain why it goes beyond previewing. So for naming, given that we now use 'result':truein our process graph, would /result be an option?

@m-mohr
Copy link
Member

m-mohr commented Feb 6, 2019

  • synchronous calls (preview/execute) are basically the way of the future. Both GEE, SentinelHub, and some services at VITO already show that meaningful information can be returned within the timing constraints of a synchronous call.

Good point.

  • asynchronous calls (batch jobs) are also needed, but especially for backends that support UDF's, where the performance of the process graph is out of the control of the backend. I don't see it as a must-have feature for every backend implementation.

So how does the user know that UDFs may not be the best options in synchronous calls?

  • web services for viewing are also very relevant, but there I've been thinking about supporting it also in a stateless manner, where the process graph is sent along with each request in a custom parameter. This way of working is also supported by SentinelHub. It allows a backend to work without requiring user management or state.

I personally don't like those ridiculously long URLs, but could still be something we want to standardize nevertheless, so could be worth a separate issue.

Now all of the above is just context to explain why it goes beyond previewing. So for naming, given that we now use 'result':truein our process graph, would /result be an option?

I'm sure I considered this option when renaming the endpoint and rejected it for some reason, but at the moment I think that could be an option. It seems consistent with the jobs, too (POST /jobs/:id/results vs POST /result). Though it would be even more intuitive when renamed to batch_jobs or so.

@m-mohr m-mohr added this to the v0.4 milestone Feb 6, 2019
@jdries
Copy link
Author

jdries commented Feb 7, 2019

The user will have to learn either by documentation or by clear error messages when to use what endpoint. Unfortunately, a call that works synchronously on one backend, might not work on another one, and there's not much we can do about that. (Except that 'slow' backends will simply not be used a lot by people looking for interactive queries.)

@m-mohr
Copy link
Member

m-mohr commented Feb 7, 2019

Okay, I renamed it to /result in dev and changed the method name in the client dev guidelines to computeResult().

@m-mohr m-mohr closed this as completed Feb 7, 2019
m-mohr added a commit that referenced this issue Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants