apache / rocketmq-client-go

Apache RocketMQ go client
https://rocketmq.apache.org/
Apache License 2.0
1.29k stars 415 forks source link

[ISSUE #3369][FEAT]create topic with broker addr or name or none #1087

Open Tomilla opened 1 year ago

Tomilla commented 1 year ago

What is the purpose of the change

Create Topic with optional argument:

Brief changelog

  1. implement GetBrokerClusterInfo of Admin interface. in the meanwhile, there are integer type keys in RocketMQ Java Server response of GetBrokerClusterInfo, Golang JSON library cannot unmarshal them directly. So I fixed the JSON didn't conform to the JSON specification (JSON keys should always be strings, don't use int as a key).

https://github.com/apache/rocketmq-client-go/blob/7eedaf948c6076c716972724782f4837e1f9caa6/admin/admin.go#L39

  1. introduce another CreateTopic implementation like sarama, without brokerAddr as input.

https://github.com/apache/rocketmq-client-go/blob/7eedaf948c6076c716972724782f4837e1f9caa6/admin/admin.go#L158

  1. add optional flag for createTopic to avoid override same name topic configuration. for back compatibility, it is false by default.

Verifying this change

I've update the examples/admin/topic/main.go file, which demonstrates all new features such as:

furthermore, I also wrote UnitTest and BenchmarkTest for JSON integer key rectify function.

Related to https://github.com/apache/rocketmq/issues/3369

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

Tomilla commented 1 year ago

Compare this to @Slideee PR (https://github.com/apache/rocketmq-client-go/pull/1006), which tried to import a go-version JavaScript interpreter into the project, but that was only used in very small cases (json loading), so it is overkill for our limited needs.

My solution is green and doesn't introduce any 3rd party heavyweight modules. furthermore, since it all uses native syntax or built-in libraries, it is better performance than adding a VM layer.

Related to https://github.com/apache/rocketmq-client-go/issues/1005

HubQin commented 6 months ago

it's a nice feature!

Tomilla commented 5 months ago

@HubQin it's a nice feature!

The code for this PR has been running stably within our company for more than half a year, indicating that there is no problem with the logic of the code itself.

However, It is unclear what the maintainer @xujianhai666 thinks about this feature merge request? If there is a need for more test cases, or additional documentation or something, please let me know exactly what to do.