Workflow grpc start API: Generate instanceID if not present

[shivamkm07]

Description

Dapr Grpc API to start workflow errors out if no instanceID is given:

dapr/pkg/api/universal/workflow_test.go

Line 74 in 6b7c94f

testName: "No instance ID provided in start request",

However, when using HTTP API, instanceID is generated if empty, as expected. This causes inconsistency between HTTP and Grpc clients. This PR updates Grpc API to have the same behavior as http.

Issue reference

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Code compiles correctly
• Created/updated tests
• Unit tests passing
• End-to-end tests passing
• Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
• Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
• Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

[codecov]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (6b7c94f) 62.12% compared to head (4fb3e8f) 62.12%.

Files	Patch %	Lines
pkg/api/universal/workflow.go	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7503      +/-   ##
==========================================
- Coverage   62.12%   62.12%   -0.01%     
==========================================
  Files         245      245              
  Lines       22346    22346              
==========================================
- Hits        13883    13882       -1     
- Misses       7303     7305       +2     
+ Partials     1160     1159       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[JoshVanL]

Please mark the instanceID as optional in the proto definition. Are other fields optional?

dapr/dapr/proto/runtime/v1/dapr.proto

Line 1046 in 6b7c94f

string instance_id = 1 [json_name = "instanceID"];

[DeepanshuA]

Please mark the instanceID as optional in the proto definition. Are other fields optional?

dapr/dapr/proto/runtime/v1/dapr.proto

Line 1046 in 6b7c94f

string instance_id = 1 [json_name = "instanceID"];

Do we require optional here?
Default value of string is blank
Do we need to differentiate between user-set blank (but present) v/s not-present blank? I don't think we need that differentiation.

If instanceId is blank, then we just need to assign a random value to it.
I think we can use here a no-presence semantic, instead of explicit-presence.

[JoshVanL]

@DeepanshuA Fields which are optional ( instancedID in this case ) which are not required by the client in the API request should always be marked as optional in protobuf. It signals to the user what is required of them to make use of the API. If an empty string is not a valid instance ID I strongly recommend also erroring to follow proper schema validation.

[artursouza]

We don't use optional in Dapr protos AFAIK. I vote for consistency. This change should define optional fields consistently with the other Dapr APIs. If we want to adopt the "optional" feature in proto, we should do it consistently in all the API.

[mukundansundar]

@DeepanshuA Fields which are optional ( instancedID in this case ) which are not required by the client in the API request should always be marked as optional in protobuf. It signals to the user what is required of them to make use of the API. If an empty string is not a valid instance ID I strongly recommend also erroring to follow proper schema validation.

We do not set optional keyword for any other "optional" fields that we already have in the protos.
I believe we can leave it as it is for this PR.

We decided to keep consistency of not using "optional" in the API.