Codecov Report

Merging #3695 (a7eb1fb) into master (aaaeb8c) will increase coverage by 0.16%.
The diff coverage is 81.92%.

❗ Current head a7eb1fb differs from pull request most recent head 8f92600. Consider uploading reports for the commit 8f92600 to get more accurate results

@@            Coverage Diff             @@
##           master    #3695      +/-   ##
==========================================
+ Coverage   62.75%   62.92%   +0.16%     
==========================================
  Files         100      100              
  Lines        9340     9423      +83     
==========================================
+ Hits         5861     5929      +68     
- Misses       3016     3028      +12     
- Partials      463      466       +3     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aaaeb8c...8f92600. Read the comment docs.

@dmitsh We need an issue to add this to Redis and another issue to add E2E tests.

Added do not merge label.

@dmitsh We need an issue to add this to Redis and another issue to add E2E tests.

dapr/components-contrib#1211
https://github.com/dapr/components-contrib/issues/1212

@artursouza please do not merge until my review. thanks.

As discussed with @artursouza, the status of these APIs should be alpha. With this change, we can leave the feature flag out.

For HTTP:

v1.0-alpha1/state.

For gRPC:

We can either introduce a new proto v2 with the new method, or we can add the new method to the existing v1 proto with a naming convention, ie. rpc QueryStateAlpha1(QueryStateRequest) returns (QueryStateResponse) {}.

/cc @msfussell @pkedy for thoughts.

For gRPC, I like having an operation suffix and not creating a V2 for preview features. Then we would remove the preview methods later because we don't care about backward compatibility for preview features. I think we should stick to the idea of only versioning schemas/protos when there are non-backward compatible changes to V1 APIs.

This proposal makes sense in that;

1. Eventually the stable "final" API for HTTP will be http://localhost:3500/v1.0/state/query. This is non-breaking since this is an additive method to the state API. It is not required to be implemented by all stores.
2. Temporarily the API for testing in the v1.5 release will be for HTTP http://localhost:3500/v1.0-alpha1/state/query and this applies to only using the query API and the existing state APIs are not affected. After this is tested then move to point 1)
3. SDKs will have to do some API manipulation between point 1) and point 2) Temporarily. No SDKs will support this query API until v1.6 release.

As discussed with @artursouza, the status of these APIs should be alpha. With this change, we can leave the feature flag out.

For HTTP:

v1.0-alpha1/state.

For gRPC:

We can either introduce a new proto v2 with the new method, or we can add the new method to the existing v1 proto with a naming convention, ie. rpc QueryStateAlpha1(QueryStateRequest) returns (QueryStateResponse) {}.

/cc @msfussell @pkedy for thoughts.

addressed

For gRPC, I like having an operation suffix and not creating a V2 for preview features. Then we would remove the preview methods later because we don't care about backward compatibility for preview features. I think we should stick to the idea of only versioning schemas/protos when there are non-backward compatible changes to V1 APIs.

It should not be v2 but v1alpha1 instead since it is an API that is not fully supported in v1 yet. The suffix hurts readability IMO.

Congrats on this PR and contribution @dmitsh!