batch: add activity reset and update-options

[spkane31]

What changed?

BatchWorkflow now supports resetting activities and updating options.

Why?

Support future cli work to support temporal activity {batch,update-options} --query ...

How did you test it?

• built
• run locally and tested manually
• covered by existing tests
• added new unit test(s)
• added new functional test(s)

Potential risks

This does not change previous behavior

[bergundy]

Didn't take a close look at the tests, left some comments about the implementation.

[bergundy]

Please merge the API change before the server PR.

[bergundy]

Good question, we should be consistent across all batch jobs with our use of identity.

[spkane31]

Added for these two, the rest of the options do not have an identity in the proto definition. I will add that in a later PR

[bergundy]

I think this is fine but you should know that using a nested proto struct in workflow input doesn't use proto JSON serialization, instead it uses plain JSON serialization which may yield unexpected results.

If we were writing this from scratch, the right thing to do here is to define a proto struct for the workflow input and reuse the request structs for each of these actions.
You may want to serialize these proto messages yourself and bypass the SDK's serialization.

[spkane31]

Chatted w/ Yuri on this. We already use the proto struct for FieldMask without custom serialization in other batch commands so we should be fine here.

[bergundy]

I would test that every field in these message are properly serialized.
Generally, what we've done here is bad practice and should be avoided in our codebase. It's not just FieldMask, it's also ActivityOptions. Even if we know that all fields are properly serialized today (which I'm not convinced of), there's no guarantee that future fields will.

[gow]

fyi: There are known issues with the default protobuf json converter when the message contains oneoff fields - https://protobuf.dev/programming-guides/json/. It's recommended to use protojson to explicitly serialize/deserialize in such cases.
In this case you're fine since UpdateActivitiesOptionsParams doesn't contain any protobufs with oneoff fields. But that may change in the future if someone adds additional fields to this input.

[bergundy]

I would like the input serialization issues to be addressed before merging but otherwise LGTM. Please do not merge before addressing.

[gow]

fyi: There are known issues with the default protobuf json converter when the message contains oneoff fields - https://protobuf.dev/programming-guides/json/. It's recommended to use protojson to explicitly serialize/deserialize in such cases.
In this case you're fine since UpdateActivitiesOptionsParams doesn't contain any protobufs with oneoff fields. But that may change in the future if someone adds additional fields to this input.

[spkane31]

I would like the input serialization issues to be addressed before merging but otherwise LGTM. Please do not merge before addressing.

Created structs for these options for JSON serialization purposes.

[bergundy]

I think this is suboptimal solution. Instead it would have been better to serialize the proto structs manually or to define the workflow inputs with protos.