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

123: Support process arrays #166

Merged

Conversation

pjhartzell
Copy link
Collaborator

@pjhartzell pjhartzell commented Nov 22, 2024

Related Issue(s):

Proposed Changes:

  1. Accept a list of process definition dictionaries.
  2. And continue to accept a bare dictionary for the process definition, but emit a Deprecation warning.
  3. Some decent massaging of the README.

PR Checklist:

  • I have added my changes to the CHANGELOG or a CHANGELOG entry is not required.

- emit deprecation warning if payload process
definition is a bare dictionary
- expect payload process definition to be first
element in a list
- update existing tests fixture to use a list of
process definitions
- add a test to check for deprecation warning when
using a dict for process definition
@pjhartzell pjhartzell force-pushed the pjh/123/support-for-process-arrays branch from 7a9342e to b51ac48 Compare November 22, 2024 03:33
stactask/task.py Outdated Show resolved Hide resolved
Copy link
Member

@jkeifer jkeifer left a comment

Choose a reason for hiding this comment

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

The code is all good, there's just some minor subtleties in the error message language that I think need to be worked out to avoid communicating inaccurate messaging to users.

Note that all of this special casing and trying to work out what the error is if we just adopted something like pydantic to parse the incoming payloads into types: if that fails then pydantic can provide much better error messages with regard to the problem.

Oh look, we already have a head start on this for someday in the future: https://github.com/Element84/swoop/blob/main/src/swoop/api/models/workflows.py#L206. 😁

stactask/task.py Outdated Show resolved Hide resolved
stactask/task.py Outdated Show resolved Hide resolved
stactask/task.py Outdated Show resolved Hide resolved
@pjhartzell
Copy link
Collaborator Author

Note that all of this special casing and trying to work out what the error is if we just adopted something like pydantic to parse the incoming payloads into types: if that fails then pydantic can provide much better error messages with regard to the problem.

Oh look, we already have a head start on this for someday in the future: https://github.com/Element84/swoop/blob/main/src/swoop/api/models/workflows.py#L206. 😁

Nice. Yeah, this is something I'm interested in getting in here at some point.

@pjhartzell pjhartzell force-pushed the pjh/123/support-for-process-arrays branch from b51ac48 to 698d444 Compare November 22, 2024 14:55
@pjhartzell pjhartzell requested a review from jkeifer November 22, 2024 14:56
@pjhartzell pjhartzell merged commit 061090d into stac-utils:main Nov 22, 2024
6 checks passed
@pjhartzell pjhartzell deleted the pjh/123/support-for-process-arrays branch November 22, 2024 17:22
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.

2 participants