Adding ResourceScope to ResourceExhaustedFailure #156
Conversation
serviceerror/resourceExhausted.go
Outdated
| Message string | ||
| st *status.Status | ||
| } | ||
| ) | ||
|
|
||
| // NewResourceExhausted returns new ResourceExhausted error. | ||
| func NewResourceExhausted(cause enumspb.ResourceExhaustedCause, message string) error { | ||
| func NewResourceExhausted(cause enumspb.ResourceExhaustedCause, scope enumspb.ResourceScope, message string) error { |
There was a problem hiding this comment.
While we may think this is only used by us, this is public API at https://pkg.go.dev/go.temporal.io/api/serviceerror and we shouldn't break compatibility IMO. Can we either have a NewResourceExhaustedWithOptions or request that those building this with a scope create the struct directly?
There was a problem hiding this comment.
Okay! I thought only server and go-sdk use this function. Let me make that change.
There was a problem hiding this comment.
I have removed this argument from this function. It is simpler to create this struct directly.
There was a problem hiding this comment.
Thanks! Will approve when api repo PR merged and regen occurs and is merged into this branch
There was a problem hiding this comment.
Thanks @cretz! I have merge api changes..
What changed?
Adding a new field ResourceScope in ResourceExhaustedFailure.
This PR depends on temporalio/api#382.
Why?
We need to differentiate whether a request has been rate limited because of namespace limit or system limit.
This will help us better understand why requests are rate limited and which limits to adjust.
How did you test it?
Potential risks
None