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

feat: conform more closely to OGC Processes API spec #44

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

drewm-jpl
Copy link
Contributor

@drewm-jpl drewm-jpl commented Sep 18, 2024

Purpose

  • This PR updates the usage of the OGC Processes API python client to more closely conform to the OGC API Processes - Part 2: Deploy, Replace, Undeploy specification.

Proposed Changes

  • [CHANGE] Usage of the OGC Processes API python client to match client interface updates.

Issues

Testing

  • SPS was successfully deployed with this version of the OGC API server. Following deployment, the cwl_dag process was deployed using the OGC API and executed multiple times with different inputs. The different inputs were reflected in the DAG run that was triggered by the execution. Job status was polled using the job status endpoint. Executions/jobs were also dismissed and dismissed jobs were verified to have been deleted from the list of OGC jobs as well as from the list of DAG runs. The cwl_dag was also successfully undeployed. This process was also performed using the unity-sds-client integration of this client.

@drewm-jpl drewm-jpl marked this pull request as ready for review September 19, 2024 19:29
@drewm-jpl
Copy link
Contributor Author

@mike-gangl This PR is ready for review whenever you have time.

Copy link
Contributor

@LucaCinquini LucaCinquini left a comment

Choose a reason for hiding this comment

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

I have not tested the changes myself, only reviewed the code. If you want a more thorough check maybe Mike or Godwin should review as well.

@GodwinShen
Copy link

GodwinShen commented Sep 25, 2024

@drewm-jpl I was able to test the changes in your branch but the job completed in a failed state, the final status message I got was "Job 5d57cd23-196c-4702-aec9-c4f53544ca7d completed with status JobStatus.FAILED". Not sure if that is expected or what, I was running the echo_message.cwl with a different message passed as an input, e.g., 'cwl_args': "{'message': 'Hello Godwin!'}". I also tried running with the default "https://raw.githubusercontent.com/unity-sds/unity-sps-workflows/main/demos/echo_message.yaml" file and got the same JobStatus.FAILED state. Not sure if this is an expected result or what, is that supposed to happen?

Nevermind, it worked on my simple "Hello Godwin" test. Going to try my EMIT test app now.

Copy link
Contributor

@mike-gangl mike-gangl left a comment

Choose a reason for hiding this comment

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

LGTM! (just kidding0

Checked this out and ran the notebook. Made some updates to that for the preprocess example- 8GB -> 16GB, it was pretty slow.

One issue, not with this code, is that the route to the ogc endpoints currently cannot go through the mdps.mcp.nasa.gov... routes. Those expect a login/cognito url. The preferable path for the OGC APIs is through apigateway- we know those routes work with our authentication.

Future Improvements:

  • We'll want to add deploy process in a future ticket, i started to add that in but since those are deployed as part of venue setup, it's not critical just yet.
  • input process. The OGC endpoint has the inputs for a process listed and its type. It'd be good to add that into the process object and then be able to allow pythonic building of the process/job submission.

@mike-gangl
Copy link
Contributor

One last thing, we can turn off pre-commit checking of the Markdown files. i don't mind it being opinionated in the code, but changing README/CHANGELOG files for the sake of it isn't terribly useful. Anyone not using that pre-commit hook will probably blast those change in the next set of PRs.

@mike-gangl
Copy link
Contributor

One last thing, the checks are failing at the coveralls section because, for some reason, the job gets closed when we re-issue runs with updated changes. i'm not 100% sure why that happens, but we'll look into fixing that in the future,

@mike-gangl mike-gangl merged commit 1561539 into main Sep 27, 2024
4 of 9 checks passed
@mike-gangl mike-gangl deleted the ogc-processes-api-updates branch September 27, 2024 16:24
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.

4 participants