rocketmq-spring icon indicating copy to clipboard operation
rocketmq-spring copied to clipboard

`RocketMQProperties#nameServer` should offer a more user-friendly format

Open snicoll opened this issue 6 years ago • 9 comments

The nameServer property ultimately maps to ClientConfig#setNamesrvAddr that requires a host:port pair separated by ;

I think it would be nicer to not expose that complexity back to the configuration. Rather, the configuration should expose a List<String> where each item represent a host:port pair.

There are several advantages to this:

  • Each pair can be checked individually and an exception can be thrown if its format is invalid
  • YAML configuration is much more readable as you can use the native list format
  • Perhaps the property can be renamed to servers to express it is a list

Taking this into consideration (+ #9) the property can be configured as follows:

rocketmq:
  servers:
    - 10.0.0.1:9876
    - 10.0.0.2:9876

snicoll avatar Dec 14 '18 10:12 snicoll

This is a good suggestion, at present most of the RocketMQ users get used to define multiple name-servers with the single line property. I incline to defer this enhancement.

walking98 avatar Dec 19 '18 08:12 walking98

@snicoll Could you help to review and find out the remaining problems or trivial polish, we would release the 2.0.1, hoping including all improvements.

vongosling avatar Dec 20 '18 14:12 vongosling

@vongosling are you asking in the context of this particular issue or globally? If you're asking globally, you can reach out by email (available on my profile). If you're asking in the context of this issue, I am afraid I didn't get the request.

snicoll avatar Dec 20 '18 14:12 snicoll

@snicoll Hah, I wonder could you have any other comments in rocketmq-spring-boot-starter. I am not sure whether your colleague aclement request you to help the community review the code. Anyway, I very appreciate your guys' review help :-)

vongosling avatar Dec 20 '18 15:12 vongosling

Got it. Yes, I am working with Andy. I'll do one more pass today.

snicoll avatar Dec 20 '18 15:12 snicoll

May I ask why this issue was closed? I don't see any change on master that relates to this request.

snicoll avatar Nov 11 '19 10:11 snicoll

Hi @snicoll, you can reopen this issue, I will submit a PR to slove this issue and ensure backward compatibility.

RongtongJin avatar Nov 13 '19 08:11 RongtongJin

Thank you but I don’t have the rights to reopen it.

snicoll avatar Nov 13 '19 08:11 snicoll

@snicoll Could you help to review this improvement? YAML and list style(rocketmq.name-server[0]=XXX, rocketmq.name-server[1]=XXX. Or rocketmq.name-server= XXX) are all supported and vefified now.

vongosling avatar Nov 15 '19 02:11 vongosling