-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
@mike-gangl This PR is ready for review whenever you have time. |
There was a problem hiding this 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.
@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. |
There was a problem hiding this 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.
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. |
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, |
Purpose
OGC API Processes - Part 2: Deploy, Replace, Undeploy
specification.Proposed Changes
Issues
Testing