apache / dubbo-js

The Typescript implementation of Apache Dubbo. An RPC and microservice framework for Node.js and Web development.
https://dubbo.apache.org/
Apache License 2.0
767 stars 160 forks source link

Add zookeeper auth supports #132

Closed xusd320 closed 5 years ago

xusd320 commented 5 years ago

For Issue:130. Add zookeeper authInfo supports. Detail is here: node-zookeeper-client#addAuthInfo

hufeng commented 5 years ago

@xusd320

感谢你的pr ^_^ 因为后续我们会支持多注册中心,支持开发者很方便的自定义自己的注册中心,所有我前段时间做了注册中心的代码重构,这是目前的api

const dubbo = new Dubbo<typeof service>({
  application: {name: 'node-dubbo'},
  // 这个作为一个兼容模式存在默认为zk作为注册中心,未来我们可能支持redis,naccos
  register: 'localhost:2181',
  service,
});

所以通过这样设计就比较单薄了不方便扩展,而且会让dubbo的初始化参数爆表。

所以做了重构之后,我们现在是这样的设计:

import {Dubbo, setting, Zk} from 'dubbo2.js';
const dubbo = new Dubbo<typeof service>({
  application: {name: 'dubbo-node-consumer'},
  service,
  dubboSetting,

  register: Zk({
    url: 'localhost:2181,localhost:2182,localhost:2183',
    // others arguments, for example authInfo
  }),
});

这样注册中心的扩展性就不会影响到Dubbo的入口参数,Keep it simple.

所以希望authInfo不要挂在this.props.authInfo, 而是作为Zk的入口参数。

xusd320 commented 5 years ago

@hufeng 感谢回复和review。 重构的设计的确具有更好的抽象,我稍后在这个思路下重新提交一次,烦请您再查阅,十分感谢!

xusd320 commented 5 years ago

@hufeng 更新了pr, 自测功能正常,辛苦您再查看一下。 更新中限定了zk acl scheme 的值,依据是: https://zookeeper.apache.org/doc/r3.1.2/zookeeperProgrammers.html Builtin ACL Schemes

hufeng commented 5 years ago

@xusd320 好的,我来review下 辛苦了