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.

Sign up for GitHub

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

Changes for closer alignment with OpenEO. #477

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Changes for closer alignment with OpenEO. #477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
56d7b44
Changes for closer alignment with OpenEO.
pvretano Dec 5, 2024
b1f9a19
Changes as per ad-hoc OGC/OpenEO discussion on 05DEC2024.
pvretano Dec 5, 2024
d2d5f3d
Fix typo.
pvretano Dec 5, 2024
107726f
Typo for multi-line descriptions.
pvretano Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion openapi/schemas/processes-core/statusCode.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
type: string
nullable: false
enum:
- accepted
- created
- queued
- running
- cancelled
Copy link

Choose a reason for hiding this comment

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

Suggested change
- cancelled
- canceled

openEO uses "canceled", is it double l in OGC API Processes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

- finished
- error
- accepted
- succeeded
- failed
- dismissed
89 changes: 47 additions & 42 deletions openapi/schemas/processes-core/statusInfo.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,47 @@
type: object
required:
- id
- status
- type
properties:
id:
type: string
processID:
type: string
type:
type: string
example:
- process
- wps
- openeo
status:
$ref: "statusCode.yaml"
message:
type: string
exception:
$ref: "../common-core/exception.yaml"
created:
type: string
format: date-time
started:
type: string
format: date-time
finished:
type: string
format: date-time
updated:
type: string
format: date-time
progress:
type: integer
minimum: 0
maximum: 100
links:
type: array
items:
$ref: "../common-core/link.yaml"
allOf:
- $ref: descriptionType.yaml
- type: object
required:
- id
- status
- created
- process
- type
Copy link

Choose a reason for hiding this comment

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

Should it be required or optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted back to the required fields for OAProc and included text in the descriptions based on the value of the "type" property. If the value of "type" is "openeo" then both "created" and "process" MUST be present as per the OpenEO API definition.

Copy link

Choose a reason for hiding this comment

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

Thanks.

Generally, I think we can do better to avoid these if/then conditionals as much as possible :-)

properties:
id:
type: string
process:
$ref: process.yaml
type:
type: string
Copy link

@m-mohr m-mohr Dec 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

process type?

It should probably just describe the encoding of the process property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the description to indicate that if "type" is "openeo" then the "process" property MUST be present and its value shall be a process graph as per the OpenEO API definition. Otherwise if can be an OAProc process description, a WPS process description, a link to either of these or a link to the request that created the job (e.g. an async coverages request).

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this condition to be more explicit?

Since we have the allOf, we can make use of it.

allOf:
  - $ref: descriptionType.yaml
  - type: object
    # ... all other fields ...
  - if:
      properties:
        type: 
          const: openeo  # or 'enum' equivalent if prefered
    then:
      required:
        - id
        - type
        - status
        - process
      properties:
        process:
          $ref: "<uri-to-openeo-process-graph>"

Copy link

@m-mohr m-mohr Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm wondering whether OGC API - Processes should just define the framework and not have direct dependencies to openEO due to the issue that there are multiple versions of the openEO API...

As long as openEO follows the framework, I guess it's fine, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm expecting that anyone that wants to do strong schema validation would actually use the reference openEO schemas rather than depend on OGC API, so it's fine if we don't define field requirements of openEO explicitly here.

However, if we do so, I don't see much point to have the long description either for specific frameworks. Saying in the description that process is required when type: openeo doesn't actually enforce or validate anything.

Copy link

@m-mohr m-mohr Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I think the spec should not include specifics about openEO. Maybe as an appendix only.

example:
- process
- wps
- openeo
status:
oneOf:
$ref: "statusCode.yaml"
message:
Copy link

Choose a reason for hiding this comment

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

Suggested change
status:
oneOf:
$ref: "statusCode.yaml"
status:
oneOf:
$ref: "statusCode.yaml"

Looks invalid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

type: string
exception:
$ref: "../common-core/exception.yaml"
created:
type: string
format: date-time
started:
type: string
format: date-time
finished:
type: string
format: date-time
updated:
type: string
format: date-time
progress:
type: integer
minimum: 0
maximum: 100
links:
type: array
items:
$ref: "../common-core/link.yaml"
Loading