zyearn commented on pull request #1529: URL: https://github.com/apache/incubator-brpc/pull/1529#issuecomment-907625668
> @zyearn 已经根据反馈修改了代码。有几个地方和你确认一下。 > > 1. Init用命名服务初始化的重载函数中，我还是判断了HTTP协议，因为如果是`file://` 或者`consul://` ，那么参数中的ns_url进行ParseHostname无意义。在InitSingle中没有判断协议了。 > 2. 当前的实现如果初始化的URL中是机器IP，调用ParseHostname（间接调用hostname2ip）也会解析成功，最后把ip存到 _hostname中，不知道是否需要额外规避掉这种情况，让 _hostname只存储主机名或者网络域名？ 1. 这样的话两处代码就不统一了。另外_hostname这个名字在channel中改成_service_name是不是更通用一些(https://github.com/apache/incubator-brpc/blob/master/src/brpc/naming_service.h#L53)？然后在HTTP协议中把_service_name作为hostname，在别的协议中可能担任另外的作用（也可以不用，例如你说的file/consul），这样似乎就不用判断协议了。 2. 如果按照1改成_service_name的话，就不存在这个问题了，因为这里预期存的是用户的输入。域名就存域名，ip的话就存ip What do you think? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org
