feat: add utility methods for primitive clients#424
Conversation
nborges-aws
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nborges-aws
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nborges-aws
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
nborges-aws
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
✅ No Breaking Changes DetectedNo public API breaking changes found in this PR. |
| target: str, | ||
| failed: Set[str], | ||
| wait_config: Optional[WaitConfig] = None, | ||
| error_field: str = "statusReasons", |
There was a problem hiding this comment.
why was this default chosen?
There was a problem hiding this comment.
The majority of CP resources use statusReasons as the error detail field, which is why I set it as the default. The few primitives whose CP resources use failureReason are set as such at the method level
| poll_interval: Seconds between status checks. Default: 10. Must be >= 1. | ||
| """ | ||
|
|
||
| max_wait: int = 300 |
There was a problem hiding this comment.
What happens if one function requires different max_waits than another? For example, if create_memories takes longer than 5 minutes usually, then create_memories_and_wait would usually fail with these defaults.
There was a problem hiding this comment.
If a *_and_wait method requires a larger window than the standard, the default max_wait can be set at the method level. Right now all the _and_wait methods pass None if the caller does not provide a WaitConfig. But if necessary for a specific case we could instead pass in a WaitConfig we define.
| """ | ||
|
|
||
| max_wait: int = 300 | ||
| poll_interval: int = 10 |
There was a problem hiding this comment.
should we do exponential backoffs? (https://docs.aws.amazon.com/prescriptive-guidance/latest/cloud-design-patterns/retry-backoff.html)
There was a problem hiding this comment.
Same answer as above. As for the exponential backoff, I think it would be good to have. Should we include that as an optional parameter or a default behavior?
2 participants
Description of changes:
Adding utility methods used in new primitive clients. Releasing utilities first so the PRs adding primitive clients can be done in parallel, as they all take dependency on these methods. The changes in this PR include:
*_and_waitmethods across primitivesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.