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

Add validation requirements class. #423

Closed
wants to merge 1 commit into from

Conversation

pvretano
Copy link
Contributor

@pvretano pvretano commented Jul 8, 2024

Closes #101.

[%metadata]
identifier:: /req/process-validate/op

The server SHALL support the HTTP POST operation at the path `/process/{processID}/validate`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be /process/{processID}/validation

@fmigneault
Copy link
Contributor

IMO, we should focus on integrating the quotation and billing definitions that were already developed (though unpublished) with @pvretano over adding a new validation endpoint. The validation does essentially the same thing as requesting the quote using a pseudo-execution request, but quoting adds the additional estimate and ID for tracking of the original inputs submitted which can be used for the later execution without replicating the request/inputs again to actually run it.

I can dig up the old definitions we started working on. It looked something like the following:

OGC API - Processes - Quotation v2

OGC API - Processes - Billing v2

Other references:

@jerstlouis
Copy link
Member

jerstlouis commented Jul 8, 2024

I agree that validation is essentially the same as quotation/estimates.

Something that could also be considered in the context of the Collection Output "on-demand" processing / GeoDataCube is essentially providing a cost per cell processed, or cost per x cells processed (which could correspond for example to a 64k cells packet for 256x256 coverage tiles, or a 59,293 sub-zones ISEA3H DGGS relative depth 10 hexagonal data packet).

In this case you don't provide a specific input Area/Time/Resolution of interest (because the actual input data may be several petabytes), but just want to know at what rate you incur costs as you make data requests which trigger processing on-demand.

Also all of this could be considered in the context of submitting an ad-hoc workflow or execution request (e.g. POST to /workflows) before actually executing it, which also is about a first "validation".

This validation-only "execution" mode is also very useful to validate nested remote processes as part of a larger distributed workflow.

@pvretano
Copy link
Contributor Author

pvretano commented Jul 8, 2024

@fmigneault @jerstlouis quotation and billiing is not something that would be in the core but rather something that would exist in another part of the suite of standards ... OGC API - Processes - Part X: Quotation and Billing.

Also, I am not sure I would explicitly tie the capability to validate a request to quotation and billing. The first step in executing a request, or getting a quotation, is to validate the request. All this PR does is simply expose an endpoint to allow that validation step to be performed independent of the next step (e.g. execution, quotation, etc.). This is what Issue #101 is requesting. If you now think this is not really necessary then I will delete this PR and close #101.

We can create a new issue (although one might already exist) to create a new part of the standard dealing with quotation and billing.

@jerstlouis
Copy link
Member

@pvretano I would very much appreciate this "validate" thing in the core.

It's something that's very useful for workflows (to facilitate integratingprocesses implementing only Core in workflows), and it's something that Part X: Quotation and Billing could extend with the extra capabilities.

I think we're saying that we should just think ahead in terms of Part X: Quotation and Billing integrating with this "don't actually execute the workflow" validating capability, in terms of end-points / parameters.

@fmigneault
Copy link
Contributor

I think it is acceptable to have this capability in Core. I agree it can be useful. Quotation can reuse the capability for the validation prior to the estimation.

However, using /processes/{processID}/validation is still problematic for workflows.
I just hope we don't get to the point where all those are needed:

  • /processes/{processID}/validation
  • /processes/{processID}/quotation
  • /processes/{processID}/execution
  • /workflow/{workflowID}/validation
  • /workflow/{workflowID}/quotation
  • /workflow/{workflowID}/execution

@jerstlouis
Copy link
Member

My initial suggestion was a query parameter for the execution mode...

(sync / async), validation, quotation

hopefully cutting those end-points by 3 :)

If ad-hoc workflow execution is by POSTing a workflow to /processes with some query parameter indicting ad-hoc workflow execution rather than deployment then wouldn't need /workflows/.

@fmigneault
Copy link
Contributor

ad-hoc workflow execution is by POSTing a workflow to /processes with some query parameter indicting ad-hoc workflow

That is an interesting suggestion.
I think ah-hoc validation on POST /processes could make sense.
Not 💯 sold on the execution though. Might be more confusing when Part 2 is supported if the same endpoint that normally creates a process can also create a job for async. That will feel even less RESTful than the current POST /processes/{processID}/execution.

@pvretano
Copy link
Contributor Author

pvretano commented Jul 8, 2024

@fmigneault @jerstlouis nope! I don't think overloading the POST method at the /processes endpoint so that one way it deploys a process and the other way it does workflow validation is a good idea at all!

Again, a workflow is JUST another "execution unit" and I don't think "ad-hoc workflow execution" is a strong enough use case to upend everything just to to accomodate it ... at least not by overloading or redefining the current behaviour of the /processes endpoint. We need to think about this a bit more!

If it turns out that workflows are a special enough resource, then I would much prefer that we define a /workflows enpoint where the full suite of CRUD methods are defined in service of workflows.

@fmigneault
Copy link
Contributor

I agree with both points (not overloading the endpoint and no special treatment of ad-hoc over execution unit that handles all other use cases/implementations).

@jerstlouis
Copy link
Member

jerstlouis commented Jul 8, 2024

no special treatment of ad-hoc over execution unit that handles all other use cases/implementations

Right now, we have a "special treatment" in Part 1- Core for ad-execution of a single process, that is the POST to /processes/{processId}/execution with an execution request submitted as a payload, which customizes execution of the process with inputs.

Part 3: Workflows "Nested Processes" introduces the ability to execute ad-hoc workflows (chained from a root process) the same way.

I am really hoping to preserve all that functionality one way or another, for the ad-hoc execution use case.

It might not be a use case that everyone cares about, but I for one believe it is a very important use case, which facilitates discovery and integration of processes available from different deployment (where the delays associated with execution endpoints requiring credentials would be a barrier to experimenting with them in a timely manner).

@gfenoy
Copy link
Contributor

gfenoy commented Jul 9, 2024

If we agree to use the POST on /jobs as proposed here to prepare a job, then I would be in favor of using GET on /jobs/{jobid}/validation and /jobs/{jobId}/quotation, and POST on /jobs/{jobId}/execution.

@pvretano
Copy link
Contributor Author

@gfenoy @fmigneault @jerstlouis I do not agree! I need more time to consider the issue but I am working on other things with the code sprint right now. Let's discuss at the next SWG meeting.

@bpross-52n
Copy link
Contributor

SWG meeting from 2024-07-22: This PR will be closed and the validation endpoint will become part of a future extension.

@bpross-52n bpross-52n closed this Jul 22, 2024
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.

Validating a process execution document without yet executing the process
5 participants