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.

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

Upgrade rocketmq for pubsub #1169

Closed
wants to merge 269 commits into from
Closed

Upgrade rocketmq for pubsub #1169

wants to merge 269 commits into from

Conversation

zach030
Copy link
Contributor

@zach030 zach030 commented Sep 25, 2021

Description

Upgrade rocketmq client with apache/rocketmq-client-go/v2 and remove dependence on third-party client.

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
  • Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

@zach030 zach030 requested review from a team as code owners September 25, 2021 15:58
@ghost
Copy link

ghost commented Sep 25, 2021

CLA assistant check
All CLA requirements met.

@daixiang0
Copy link
Member

Could share why need to upgrade?

@CodeMonkeyLeet
Copy link
Contributor

@zach030 can you also look at the build failures? It looks like the resulting go.sum is incomplete so a go mod tidy is needed

@zach030
Copy link
Contributor Author

zach030 commented Sep 27, 2021

Could share why need to upgrade?

The previous pubsub component rocketmq depends on third-party libraries. According to the opinions of the RocketMQ community, I remove it and make it purely base on apache/rocketmq-client-go

@zach030
Copy link
Contributor Author

zach030 commented Oct 6, 2021

@zach030 can you also look at the build failures? It looks like the resulting go.sum is incomplete so a go mod tidy is needed

done

pubsub/rocketmq/rocketmq.go Outdated Show resolved Hide resolved
pubsub/rocketmq/metadata.go Show resolved Hide resolved
- name: accessKey
value: "RocketMQ"
- name: secretKey
value: "12345"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Missing newline at EOF for linux text files.

@CodeMonkeyLeet
Copy link
Contributor

@yaron2 @artursouza This is a fairly significant change, have the maintainers approved this? It looks in line with previous requests such as #890

@CodeMonkeyLeet
Copy link
Contributor

@zach030 Still hitting build failures with the go.sum issues

@artursouza
Copy link
Member

artursouza commented Oct 6, 2021

This component was contributed by Alibaba.

/cc @LaurenceLiZhixin Is this change OK for Alibaba?

xj524598 and others added 24 commits December 19, 2021 00:54
Co-authored-by: xujie <xujie1@shein.com>
Signed-off-by: zach <zachchou016@gmail.com>
)

Co-authored-by: Long Dai <long.dai@intel.com>
Co-authored-by: Artur Souza <artursouza.ms@outlook.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* support tracestate can be propagated in PubSub

* add trace state

Co-authored-by: Long Dai <long.dai@intel.com>
Co-authored-by: Ian Luo <ian.luo@gmail.com>
Co-authored-by: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* bugfix for sns topic deletion upon termination

* removed upstream github workflow files

* Update snssqs.go

* dapr bot schedule

* read and append queue attributes

* unnecessary escaping in json tag

* unexporting structs

Signed-off-by: zach <zachchou016@gmail.com>
Signed-off-by: zach <zachchou016@gmail.com>
* fix: add config subscription gr done

* unexpected comments

* fix comment

Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* Adds retry on CosmosDB Init in case of TooManyRequests error

* Use backoff v4

* missed some permanent errors

* clean up go.mod

* fix error type casting

* Add constant for HTTP 429

Signed-off-by: zach <zachchou016@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* bugfix for sns topic deletion upon termination

* removed upstream github workflow files

* wrapping errors

* fifo and naming sanitization

* fifo and name santization tests

* component carrying id

* Update snssqs.go

removed unneeded import

* Update snssqs.go

* add missing github actions file

* linting

* fix go.mod

* schedule yml file

* const max lenght, bugfix in ack

* dapr bot schedule

Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* Add certification tests for ASB Queue binding

This commit adds certification tests for the Azure Service Bus Queue
Input/Output binding. This utilizes the new certification framework
and performs a series of tests as described in the test plan readme.

The general purpose of these tests is to serve as an integration test
and as such requires an actual Azure Service Bus connection and a
Dapr sidecar.

#957

* Add to github workflow and fix formatting

* Fix dependencies

Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
Signed-off-by: zach <zachchou016@gmail.com>
* bugfix for sns topic deletion upon termination

* removed upstream github workflow files

* Update snssqs.go

* dapr bot schedule

* read and append queue attributes

* unnecessary escaping in json tag

* unexporting structs

* bugfix in policy

* bugfix in policy. merged from master

* fifo suffix as const

Signed-off-by: zach <zachchou016@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
* Fix test infrastructure setup script

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

* use more unique ACR resource name

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

Co-authored-by: Looong Dai <long.dai@intel.com>
Signed-off-by: zach <zachchou016@gmail.com>
Signed-off-by: zach <zachchou016@gmail.com>
Signed-off-by: Amit Mor <amit.mor@hotmail.com>
Signed-off-by: zach <zachchou016@gmail.com>
* Adds CosmosDB Binding authentication tests

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

* Enable Cosmos DB Binding Conformance test

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

* Initial cosmosdb binding certification plan WIP

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

* Go mod tidy

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

* Update library and go mod tidy

Signed-off-by: GitHub <noreply@github.com>
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>

* CosmosDB Binding test plan details

Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
Signed-off-by: Dmitry Shmulevich <dmitry.shmulevich@gmail.com>
Signed-off-by: zach <zachchou016@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
Signed-off-by: zach <zachchou016@gmail.com>
@codecov
Copy link

codecov bot commented Dec 18, 2021

Codecov Report

Merging #1169 (a47cc39) into master (870fedb) will increase coverage by 0.29%.
The diff coverage is 37.65%.

❗ Current head a47cc39 differs from pull request most recent head 1e939c5. Consider uploading reports for the commit 1e939c5 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1169      +/-   ##
==========================================
+ Coverage   34.95%   35.24%   +0.29%     
==========================================
  Files         149      149              
  Lines       12923    12962      +39     
==========================================
+ Hits         4517     4569      +52     
+ Misses       7922     7898      -24     
- Partials      484      495      +11     
Impacted Files Coverage Δ
tests/conformance/common.go 16.60% <0.00%> (-0.13%) ⬇️
pubsub/rocketmq/rocketmq.go 27.81% <31.15%> (+27.81%) ⬆️
pubsub/rocketmq/metadata.go 81.81% <81.81%> (ø)

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 7a631ef...1e939c5. Read the comment docs.

@zach030
Copy link
Contributor Author

zach030 commented Dec 18, 2021

@yaron2 I find it's quite confused for me to fix conflict while rebasing. So can I checkout a new branch and open another PR?

@yaron2
Copy link
Member

yaron2 commented Dec 18, 2021

@yaron2 I find it's quite confused for me to fix conflict while rebasing. So can I checkout a new branch and open another PR?

Yep go for it

@zach030 zach030 mentioned this pull request Dec 18, 2021
3 tasks
@zach030 zach030 closed this Dec 18, 2021
@zach030
Copy link
Contributor Author

zach030 commented Dec 18, 2021

new pr #1383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge PR is not ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet