Add doc comments to new struct fields for serialization context#2324
Conversation
| Err error | ||
| // Namespace is the Temporal Namespace containing the Workflow Execution. | ||
| // This field is required. | ||
| Namespace string |
There was a problem hiding this comment.
Why do we allow setting the namespace different from the one to which the client is bound?
There was a problem hiding this comment.
Good question, had to dig into this
It looks like only for CompleteActivityOptions should the namespace match the client namespace, since that is task-token based, which always uses the client namespace, whereas all of the others can target other namespaces.
There was a problem hiding this comment.
whereas all of the others can target other namespaces.
I recall that server deprecated cross-namespace workflow commands (see #2267). I know that this is change is on the client, so it's different. Does enable some hypothetical client that is not attached to a specific namespace to send async completions? If a client is always attached to one namespace, I don't think we should add these namespace fields.
There was a problem hiding this comment.
Updated comments to not encourage cross-namespace things
2 participants
What was changed
Add comments for each field in the new structs I added in #2225.
Why?
New changes from the PR mentioned above is causing CI in #2232 to fail, it's probably easiest if I just create this PR instead of suggesting changes, as these are all with new structs I added recently
Note
Low Risk
Documentation-only updates to exported option structs; no functional or behavioral changes expected.
Overview
Adds detailed godoc to the activity completion/heartbeat option structs used by
CompleteActivity*WithOptionsandRecordActivityHeartbeat*WithOptions, documenting required vs optional fields and how each field affects serialization/external-storage context.Also adds prominent warnings in the public
clienttype aliases that serialization-context values (e.g.,ActivityType,WorkflowType,TaskQueue,WorkflowID) are not validated by the SDK and incorrect values may break codecs that depend on them.Reviewed by Cursor Bugbot for commit a3a86f9. Bugbot is set up for automated code reviews on this repo. Configure here.