Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature: support dapr state api #377

Merged
merged 11 commits into from
Jan 13, 2022
Merged

feature: support dapr state api #377

merged 11 commits into from
Jan 13, 2022

Conversation

zach030
Copy link
Contributor

@zach030 zach030 commented Dec 28, 2021

What this PR does:

feature:

  1. support dapr state api
  2. integrate dapr API with default_api
  3. add some converter codes
  4. implement QueryStateAlpha1 and add some missing error-msg consts

Which issue(s) this PR fixes:

Fixes #361

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

@seeflood
Copy link
Member

Thanks for your contribution !
Please check and fix the CI failure.
@wenxuwan Could u help review this PR?

@wenxuwan wenxuwan self-requested a review December 29, 2021 02:13
@wenxuwan
Copy link
Member

Thanks for your contribution ! Please check and fix the CI failure. @wenxuwan Could u help review this PR?

ok

@wenxuwan
Copy link
Member

@zach030 please fix the failed ci first

@zach030
Copy link
Contributor Author

zach030 commented Dec 29, 2021

@zach030 please fix the failed ci first

Yes. But I don't know the reason of CI failure now 🥲

@wenxuwan
Copy link
Member

@zach030 please fix the failed ci first

Yes. But I don't know the reason of CI failure now 🥲

should be cause by cel-go version, you can check cel-go version in the go.sum

@zach030
Copy link
Contributor Author

zach030 commented Dec 30, 2021

@zach030 please fix the failed ci first

Yes. But I don't know the reason of CI failure now 🥲

should be cause by cel-go version, you can check cel-go version in the go.sum

我查了一下，我的这次修改引入了对dapr的依赖：github.com/dapr/dapr v1.5.1
dapr依赖的cel-go的版本是github.com/google/cel-go v0.7.3 ，而mosn依赖的版本是github.com/google/cel-go v0.5.1

然后我对比了一下v0.7.3和v0.5.1 google/cel-go@v0.5.1...v0.7.3
common/types/duration.go 两个版本里有比较大的差异
image

@wenxuwan
Copy link
Member

@zach030 please fix the failed ci first

Yes. But I don't know the reason of CI failure now 🥲

should be cause by cel-go version, you can check cel-go version in the go.sum

我查了一下，我的这次修改引入了对dapr的依赖：github.com/dapr/dapr v1.5.1 dapr依赖的cel-go的版本是github.com/google/cel-go v0.7.3 ，而mosn依赖的版本是github.com/google/cel-go v0.5.1

然后我对比了一下v0.7.3和v0.5.1 google/[email protected] common/types/duration.go 两个版本里有比较大的差异 image

替换成0.5.1看dapr那边会不会报错

@codecov
Copy link

codecov bot commented Jan 1, 2022 •
edited
Loading

Codecov Report

Merging #377 (d1f3b25) into main (a9b4598) will decrease coverage by 3.40%.
The diff coverage is 22.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
- Coverage   57.61%   54.21%   -3.41%     
==========================================
  Files         107      108       +1     
  Lines        5762     5921     +159     
==========================================
- Hits         3320     3210     -110     
- Misses       2105     2382     +277     
+ Partials      337      329       -8
Impacted Files Coverage Δ
pkg/grpc/dapr/dapr_api_state.go 0.00% <0.00%> (ø)
pkg/grpc/default_api/api.go 73.06% <ø> (-2.74%) ⬇️
pkg/runtime/state/compatibility.go 100.00% <ø> (+25.45%) ⬆️
pkg/grpc/default_api/api_state.go 80.18% <80.18%> (ø)
components/configstores/apollo/change_listener.go 74.00% <0.00%> (-6.00%) ⬇️
components/rpc/invoker/mosn/channel/xchannel.go 80.64% <0.00%> (-3.23%) ⬇️

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 a9b4598...d1f3b25. Read the comment docs.

@zach030
Copy link
Contributor Author

zach030 commented Jan 4, 2022

@wenxuwan already fixed ci failure

@wenxuwan
Copy link
Member

wenxuwan commented Jan 4, 2022

@wenxuwan already fixed ci failure

ok， i will review asap

wenxuwan
wenxuwan previously approved these changes Jan 10, 2022
View reviewed changes
Copy link
Member

@wenxuwan wenxuwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

seeflood
seeflood requested changes Jan 11, 2022
View reviewed changes
go.mod Outdated Show resolved Hide resolved
seeflood
seeflood requested changes Jan 11, 2022
View reviewed changes
pkg/grpc/dapr/dapr_api_state.go Outdated Show resolved Hide resolved
@zach030
Copy link
Contributor Author

zach030 commented Jan 11, 2022

@seeflood 移除了加密部分的代码，还有对 github.com/dapr/dapr v1.5.1的依赖，目前看来没有版本冲突的问题了

@seeflood
Copy link
Member

FYI, I removed some unused code,e.g. pkg/converter/state.go

seeflood
seeflood approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@seeflood seeflood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

wenxuwan
wenxuwan approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@wenxuwan wenxuwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wenxuwan wenxuwan merged commit d75608f into mosn:main Jan 13, 2022
seeflood added a commit to seeflood/layotto that referenced this pull request Feb 10, 2022
@seeflood
feature: support dapr state api (mosn#377)
7eb35e5 
* dapr/state:add get/save state dapr api

Signed-off-by: zach <[email protected]>

* dapr/state:add query/delete state

Signed-off-by: zach <[email protected]>

* dapr/state_api:replace cel-go with v0.5.1

Signed-off-by: zach <[email protected]>

* dapr/state_api:fix state api ut

Signed-off-by: zach <[email protected]>

* dapr/state_api:init slice with cap

Signed-off-by: zach <[email protected]>

* dapr/state_api: remove encrypt dependency

Signed-off-by: zach <[email protected]>

* remove unused code

* add comments

Co-authored-by: seeflood <[email protected]>
# Conflicts:
#	pkg/grpc/dapr/dapr_api_state.go
#	pkg/grpc/dapr/dapr_api_unimplement.go
#	pkg/grpc/default_api/api.go
#	pkg/grpc/default_api/api_state.go
seeflood added a commit to seeflood/layotto that referenced this pull request Feb 10, 2022
@zach030 @seeflood
feature: support dapr state api (mosn#377)
f014696 
* dapr/state:add get/save state dapr api

Signed-off-by: zach <[email protected]>

* dapr/state:add query/delete state

Signed-off-by: zach <[email protected]>

* dapr/state_api:replace cel-go with v0.5.1

Signed-off-by: zach <[email protected]>

* dapr/state_api:fix state api ut

Signed-off-by: zach <[email protected]>

* dapr/state_api:init slice with cap

Signed-off-by: zach <[email protected]>

* dapr/state_api: remove encrypt dependency

Signed-off-by: zach <[email protected]>

* remove unused code

* add comments

Co-authored-by: seeflood <[email protected]>
@seeflood seeflood mentioned this pull request Mar 4, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MoonShining MoonShining MoonShining left review comments

@seeflood seeflood seeflood approved these changes

@wenxuwan wenxuwan wenxuwan approved these changes

Assignees

@wenxuwan wenxuwan

Labels
cla:yes size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Dapr API
4 participants
@zach030 @seeflood @wenxuwan @MoonShining