Skip to content

fix: Converting configuration API response from an array to dictionary #4695

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
Closed
wants to merge 11 commits into from
Closed

fix: Converting configuration API response from an array to dictionary #4695

wants to merge 11 commits into from

Conversation

beiwei30
Copy link
Member

@beiwei30 beiwei30 commented Jun 1, 2022 •
edited
Loading

Signed-off-by: Ian Luo ian.luo@gmail.com

Description

fix issue #4556

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:

@beiwei30 beiwei30 requested review from a team as code owners June 1, 2022 10:03
@beiwei30
Copy link
Member Author

beiwei30 commented Jun 1, 2022

Have to wait for dapr/components-contrib#1771 merged. Pls. don't merge for now.

@LaurenceLiZhixin pls. take a look.

@beiwei30 beiwei30 mentioned this pull request Jun 1, 2022
Closed
@yaron2
Copy link
Member

yaron2 commented Jun 1, 2022

@beiwei30 see build errors:

Error: pkg/grpc/api.go:1554:15: cannot use k (variable of type int) as type string in map index
Error: pkg/grpc/api.go:1577:9: cannot use k (variable of type int) as type string in map index

@LaurenceLiZhixin
Copy link
Contributor

Thanks for @beiwei30 , the modification seems to me to be in the right direction. Please update conpoment version after dapr/components-contrib#1771 is merged.

@yaron2 The build errors occurs because the component not updated, which is in line with expectations :)

@yaron2
Copy link
Member

yaron2 commented Jun 2, 2022

The build is failing now due to another issue which is unrelated. @ItalyPaleAle will submit a PR to fix.

@yaron2
Copy link
Member

yaron2 commented Jun 2, 2022

@beiwei30

Error: pkg/grpc/api.go:1554:15: cannot use k (variable of type int) as type string in map index
Error: pkg/grpc/api.go:1577:9: cannot use k (variable of type int) as type string in map index

@beiwei30 beiwei30 added this to the v1.9 milestone Jun 6, 2022
@beiwei30
Copy link
Member Author

beiwei30 commented Jun 6, 2022 •
edited
Loading

@beiwei30

Error: pkg/grpc/api.go:1554:15: cannot use k (variable of type int) as type string in map index
Error: pkg/grpc/api.go:1577:9: cannot use k (variable of type int) as type string in map index

@yaron2 This is expected. Because the change made on component-contrib side (dapr/components-contrib#1771) has not been merged yet. Once it's done, I need to bump up component-contrib version in order to pass compilation failure.

Right now I submit the following change to make build pass:

replace github.com/dapr/components-contrib => github.com/beiwei30/components-contrib v0.4.1-0.20220603163116-fa6cff30d029

beiwei30 added 4 commits June 6, 2022 10:43
@beiwei30
Merge branch 'master' into issue/4556
5164a28
@beiwei30
use replace to pass compilation.
58bc46e 
Signed-off-by: Ian Luo <ian.luo@gmail.com>
@beiwei30
fix compilation
a610271 
Signed-off-by: Ian Luo <ian.luo@gmail.com>
@beiwei30
fix unit test
4e6a414 
Signed-off-by: Ian Luo <ian.luo@gmail.com>
@codecov
Copy link

codecov bot commented Jun 6, 2022 •
edited
Loading

Codecov Report

Merging #4695 (bba79d1) into master (5525744) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4695      +/-   ##
==========================================
- Coverage   65.10%   65.09%   -0.02%     
==========================================
  Files         111      111              
  Lines       11597    11593       -4     
==========================================
- Hits         7550     7546       -4     
  Misses       3500     3500              
  Partials      547      547
Impacted Files Coverage Δ
pkg/grpc/api.go 72.66% <100.00%> (-0.12%) ⬇️

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 5525744...bba79d1. Read the comment docs.

@yaron2
Copy link
Member

yaron2 commented Jun 6, 2022

@beiwei30

Error: pkg/grpc/api.go:1554:15: cannot use k (variable of type int) as type string in map index
Error: pkg/grpc/api.go:1577:9: cannot use k (variable of type int) as type string in map index

@yaron2 This is expected. Because the change made on component-contrib side (dapr/components-contrib#1771) has not been merged yet. Once it's done, I need to bump up component-contrib version in order to pass compilation failure.

Right now I submit the following change to make build pass:

replace github.com/dapr/components-contrib => github.com/beiwei30/components-contrib v0.4.1-0.20220603163116-fa6cff30d029

@LaurenceLiZhixin @beiwei30 you're right, apologies :)

We will merge this change right after the code freeze (6/21) for 1.9 as we need more time to update the SDKs to match the new data structure.

This was referenced Jul 9, 2022
Closed
Merged
@pravinpushkar pravinpushkar mentioned this pull request Jul 19, 2022
Closed
@artursouza artursouza removed this from the v1.9 milestone Aug 1, 2022
@addjuarez
Copy link
Contributor

ping @beiwei30

berndverst
berndverst reviewed Aug 23, 2022
View reviewed changes
go.mod
@@ -351,6 +351,8 @@ replace (

replace github.com/eclipse/paho.mqtt.golang => github.com/shivamkm07/paho.mqtt.golang v1.3.6-0.20220106130409-e28a1db639f8

replace github.com/dapr/components-contrib => github.com/beiwei30/components-contrib v0.4.1-0.20220603163116-fa6cff30d029
Copy link
Member

Choose a reason for hiding this comment

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

Remove this please. We can make coordinate whatever change is required in the contrib repo first.

berndverst
berndverst requested changes Aug 23, 2022
View reviewed changes
Copy link
Member

@berndverst berndverst left a comment

Choose a reason for hiding this comment

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

Looks good to me, but this depends on the change in dapr/contrib being merged first, and then the version of contrib needs to be imported here I think.

This was referenced Aug 23, 2022
Closed
Merged
artursouza pushed a commit that referenced this pull request Aug 26, 2022
@berndverst @beiwei30 @mcandeia
Configuration API response to use dictionaries - based on PR #4695 (#…
683a436 
…5083)

* fix: Converting configuration API response from an array to dictionary. #4556

Signed-off-by: Ian Luo <ian.luo@gmail.com>

* use replace to pass compilation.

Signed-off-by: Ian Luo <ian.luo@gmail.com>

* fix compilation

Signed-off-by: Ian Luo <ian.luo@gmail.com>

* fix unit test

Signed-off-by: Ian Luo <ian.luo@gmail.com>

* Update http API for config dicts

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>

* gen proto

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>

* Fix http tests

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>

* import latest contrib

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>

* make modtidy-all

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>

Signed-off-by: Ian Luo <ian.luo@gmail.com>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
Co-authored-by: Ian Luo <ian.luo@gmail.com>
Co-authored-by: Marcos Candeia <marrcooos@gmail.com>
Taction added a commit to Taction/dapr that referenced this pull request Sep 5, 2022
@Taction
Merge branch 'master' into feat_add_cache_for_secret
5d216cc 
* master: (134 commits)
  Record metric for service updates (dapr#5114)
  chore: remove duplicate word in comments (dapr#5099)
  Run e2e tests in previous version of K8s on AKS to see if it's breaking Windows tests (dapr#5104)
  add {namespace} option to set consumerID
  Add tracing passthrough for bindings (dapr#5049)
  Improve component init error logs and metrics (dapr#4975)
  Improvements to E2E test apps (dapr#5042)
  Fix component name for aws dynamodb (dapr#5095)
  Fix perf tests not building (dapr#5096)
  Fix missing components in Metadata API result. (dapr#5052)
  Fixed log message (dapr#5088)
  Disable IPFS binding for now (dapr#5092)
  optimize/concurrency: pubsub concurrency map writes in runtime unit test. (dapr#5086)
  Configuration API response to use dictionaries - based on PR dapr#4695 (dapr#5083)
  correct compile error
  correct checking for actors with same id
  fix type error in print
  fix typo
  add log when actor start time and end time is not expected
  Components auto-registration (dapr#5062)
  ...

Signed-off-by: zhangchao <zchao9100@gmail.com>

# Conflicts:
#	go.mod
#	pkg/grpc/api.go
#	pkg/grpc/api_test.go
#	pkg/http/api.go
#	pkg/runtime/runtime.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berndverst berndverst berndverst requested changes

Assignees
No one assigned
Labels
do-not-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@beiwei30 @yaron2 @LaurenceLiZhixin @addjuarez @berndverst @artursouza @daixiang0