feat: payload limits validation visitor#1327
Conversation
438ac14 to
d2ddd07
Compare
d2ddd07 to
55c894c
Compare
| } | ||
|
|
||
| #[derive(Clone, Copy)] | ||
| enum LeafKind { |
There was a problem hiding this comment.
It feels like this is serving the same purpose as PayloadFieldKind which exists in the file already. Can we re-use that? Maybe it needs some extra variants. Broadly it seems like maybe some code could get shared here in general, AFAICT we don't re-use any of the existing visitor code but in principle I feel like we should've been able to extend that code to serve this purpose?
If it doesn't make sense, no prob, but, worth seeing if we can reduce LoC here
There was a problem hiding this comment.
I tried to share that specifically but both visitors have different terminal conditions and additional semantics carried with each of these enums, so I didn't refactor that. Was able to refactor and share more of the common message traversal and classification logic though. Ended with marginally less LoC but that might be due to comment add/removal.
Sushisource
left a comment
There was a problem hiding this comment.
Nice, glad that zip suggestion reduced things a lot
What was changed
Add a SDK-internal, build-time generated payload/memo limit visitor:
temporal.api.*rpc input message)PayloadLimitsValidatableimplementations to report each payload-bearing field size and classification to aPayloadLimitsSinkimplementation.PAYLOAD_LIMITS_TABLEinbuild.rsto map each payload-bearing message and field combination to a classification. The build will fail if there are any missing or stale entries. This allows us to classify new combinations as they are added to the API. Initial classification answers built from inspection oftemporal/temporalrepository.Payload,Payloads,Memo,Failure,Any,SearchAttributes, andHeaderleaf fields.PayloadLimitSinkreceives callbacks from visitor to consume the size and classification information for each visited message and field combination.validation_payload_limitsfunc to log errors and warnings and returns the first error-levelPayloadLimitViolation. Each violation carries a proto-field-name path that is relative to the root message.Approximately 370 lines of
build.rsis the classification table itself.Approximately 300 lines of
payload_limits.rsis tests.Why?
Create a foundational element that will be later integrated to allow clients and workers to validate payload limits and proactively issue WFT / AT failures.
Checklist