-
Notifications
You must be signed in to change notification settings - Fork 46
Description
Below is my feedback reading through the candidate standard (up to end of 7-Core -- will create separate issue for Sections 8 Collections+):
Abstract
- The Abstract has tables. I think it would be a better fit to move these to Section 6 Overview which is currently very small. AFAIK the Abstract is intended to be text only / easy to include in a summary description of the document
- I would also suggest not including so much "background context" about WPS in the abstract (other than e.g., one sentence that OGC API - Processes is the modern REST/JSON-based successor to WPS), and focus more on summarizing what this particular standard is. Maybe the more detailed WPS background could be a sub-section of the Overview section
- In table 1, I would call the "Result" resource "Individual output result" to clarify the distinction with "Job results"
- "Additionally, the
/jobsendpoint can be used to grant access to a list of jobs." -- I do not understand what this sentence means
Preface
The Standard specified who to “wrap” computational tasks into an executable process that can be invoked by a client application.
How?
Security Section
- It's a bit unusual that the Recommendation 1 is within the Security section rather than within the main body of the document
- Everywhere in the document it seems that there is both Identifier and Label (with identical URIs). I think we should only keep
Identifier (Label was the pre-metanorma thing).
Acknowledgements
- Please include an acknowledgement section to credit the work done as part of our GeoConnections project using the following exact language below (and other things if appropriate):
Requirements integrated within version 2.0 of this OGC API - Processes Standard was sourced from Part 3: Workflows, whose development was undertaken as part of GeoConnections 2020-2021 and 2025-2027 projects GNS20-21IFP02 (Modular OGC API Workflows) and GNS25-27IFP2 (Advancing and Implementing OGC Standards for GeoDataCubes, DGGSs, Data Stores for Digital Twins and Cartographic Symbology). Financial support provided by GeoConnections, a national collaborative initiative led by Natural Resources Canada. GeoConnections supports the modernization of the Canadian Geospatial Data Infrastructure (CGDI). The CGDI is the collection of geospatial data, standards, policies, applications, and governance that facilitate its access, use, integration, and preservation.
Conformance
- for the resources specified in the Corea (remove the a)
- The Collection Input, Remote Collections, Collection Outout, Local Filtering conformance classes are missing here from both the table and summary.
- I think we also want at least "Derived Properties", and possibly also "Filtering", as separate conformance classes (so that implementations can support Collection Input without forcing them to implement Derived Fields)
Normative References
-
WPS 2.0 is included as a normative reference . I don't think that should be? It is not a dependency of any requirement, is it?
-
Is OGC API - Common - Part 1 not included on purpose? Is this an intended change from Version 1? It seems that Version 1's Core requirement class had an explicitly dependency on OGC API - Common - Part 1: Core's "Core" requirement class
Terms and Definition
- Collection: could we update this to the latest definition in Part 2:
(in the context of OGC APIs) A set of spatiotemporal data that may be available through one or more access mechanisms defined by OGC API standard(s)
- data access mechanism -- might also synchronize that definition:
(in the context of OGC APIs) mechanism defined by an OGC API Standard to access spatiotemporal data from a collection
- process: Processes that have no process inputs represent value generators that deliver constant or random process outputs.
I would argue that a process taking no input (from the executing client) can return outputs which are neither constant nor random. e.g., it can be sensing things from the environment or from inputs not coming from the client but from hard-wired connections on the server side (e.g., I can hook up my electric piano via MIDI to an OGC API - Process and serve what I'm playing via a process taking no input from the client) - 4.1.18. process output (explicitly requested)
A process output with identifier {outputID} that is explicitly specified in the “outputs” section of an execute request.
This is confusing here, because it seems to imply that if you omit theoutputscompletely (defaulting to all output), you will not get a.../results/{outputId}for each output? I don't think that should be the case. I would personally not include 4.1.18. process output (explicitly requested) and 4.1.19. process output (implicitly requested) in the Terms and Definitions, but include those clarifications in the execution / output sections. - 4.1.20. remote collection -- I would try to use the positive here instead of saying what it is not (local)... e.g., "available from an external OGC API deployment (different from the deployment whose process is being executed)..."
Conventions
- In all the OGC standards that I'm editing now (since OGC API - Maps) we use HTTPS URIs everywhere for everything (including conformance classes), following a request by the OAB to do so. The argument was that several organizations have security policy that prevent them from accessing HTTP URIs completely. I realized that this practice is not being consistently applied across the different OGC standards being published and that's a bit puzzling -- can we apply it here for this standard?
Core
- The URL for accessing status information is delivered in the HTTP header location. -- Please make this HTTP response header and uppercase L:
Location - In Figure 1, the Process resource box has
mode,response,subscriber-- are these still a thing? I don't see them in process.yaml or processSummary.yaml or descriptionType.yaml (I do seejobControlOptions,links,versionthat is missing though).subscriberis for the execution request, but this is the process description, right? Alsoversionis marked as required, is it really necessary to make version required? Forced metadata is a recipe for bad metadata. - As I mentioned above, the dependency on OGC API - Common - Part 1 was removed -- can we add it back?
- The mapping in Table 9 to Common req. classes is not right, and the req. class URIs use a /2.0 where that should be 1.0. The API Landing Page, API Definition and Conformance is defined in the landing page conformance class -- not core:
http://www.opengis.net/spec/ogcapi-common-1/1.0/req/landing-page, - Recommendation 2 is missing the "Statement" or Part (A, B...)
- The text regarding content negotiation and the following notes are very confusing:
If multiple API definition formats are supported by a server, use content negotiation to select the desired representation.
NOTE: Two common approaches are: (... An additional path for each encoding ... An additional query parameter)
This seems to suggest that the approaches to content negotiation are the suffixes and/or query parameters INSTEAD of the proper HTTP content negotiation via the Accept: request header
- Example — Requirements class response document -- this still has 1.0 example conformance classes listed
- using the
profileparameter -- please clarify query parameter to distinguish from path parameter (everywhere)... also for 7.8 limit query parameter below - 7.7 specified the Link header with rel="profile" in the request headers. -- Is this really a thing including
Linkheader as a request header? I have never seen that before and Link is normally aresponseheader... The Link rel=profile is normally a response header for the server to tell you which profile it actually applied (as stated below). Seems like that is at least one too many/unnecessary ways to do the profile negotiation (I would stick to only theprofilequery parameter and theAccept-Profile:request header myself) - 7.10. Retrieve a process list -- implmentation missing an e
- 7.10.1.2 (Query) Parameter limit -- Recommendations 10, 11, 12 about
limitseem like they should not be recommendations but describe the basic way how the paging works, with the limit query parameter being required. While I would argue that for the use cases of servers supporting less than 100 processes paging should not be mandatory to be supported and the entire thing should be a recommendation, it does not make sense to me to make these 3 specific things only recommendations. Make the whole thing optional, or make the whole thing (with the exception of theprevlink) requirements. It would also help in my opinion to collapse some of these separate statement into multiple parts of the same requirement. - Recommendations 16 and 19 make recommendations for CLIENTS. Since Section 2 conformance says The standardization targets of all conformance classes are “Web APIs.” (and not clients), then perhaps these recommendations could be changed to an IMPORTANT: note rather than formal recommendations.
- Requirement 30: The content of response body SHALL be the requested process output value in the default output format. (when no
Accept:header is used)
Should this not be the specified output format specified in the execution request, which I think we still have? (is there actually an explicit "default" format specified in the description?) - Permission 9: has a pre-metanorma left-over saying "part"
- Requirement 39 ( /req/core/job-op ) -- The server SHALL support the HTTP GET operation at the path /jobs/{jobID}. -- This is missing conditions to clarify that this is not required for synchronous execution
- Requirement 40 -- This requirement is not set up properly in metanorma, with Statement appearing multiple times instead of A, B, C... There's a typo
inidicate. It's also missing the conditions like req. 39 to be only required for Async. - Requirements 41, 42, 43, 44, 45 ( /req/core/job-results-op ) are missing conditions about being only required for Async. -- Would it make sense to move all the Async stuff into an Asynchronous Execution / Job management requirements class? That would still not prevent Sync execution still creating a job for servers that want to do that. It would greatly help to keep the Core req. class simpler / smaller.
- Requirement 44: /req/core/job-results-param-outputs-response -- Only the outputs with identifers specified by the optional outputs parameter SHALL be included in the response. -- What is this talking about? Is it talking about the
outputsproperty in the execution request? If so, it would help to clarify this by not using the word parameter here (sayproperty) and mention of the execution request, and also clarifying: "(or all outputs when not specified)". - Requirement 50 The request is executed on a running job with valid job identifier jobID. -- missing a
d