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

Implement getConfigs method for ConfigClient #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

raphaelsoul
Copy link

No description provided.

@raphaelsoul raphaelsoul changed the base branch from dev to master April 21, 2020 13:26
@raphaelsoul
Copy link
Author

raphaelsoul commented Apr 21, 2020

实现了一下getConfigs方法 我自己用如下代码做workaround。
看情况close吧 dev分支差异蛮大的

ps: 没找到md说明如何接收社区PR

const ClientWoker = require("nacos-config/dist/client_worker").ClientWorker

ClientWoker.prototype.getConfigs = async function (appName) {
    this.debug('calling getConfigs, appName %s', appName);

    let content;
    const key = this.getSnapshotKey("", "") + "/" + appName;
    // TODO 优先使用本地配置
    appName = appName || this.options.configuration.innerConfig.appName
    try {
        content = await this.httpAgent.request(this.apiRoutePath.GET, {
            data: {
                dataId: "",
                group: "",
                appName,
                tenant: this.namespace,
                search: "accurate",
                pageNo: 1,
                pageSize: 200
            },
        });
    }
    catch (err) {
        const cache = await this.snapshot.get(key);
        if (cache) {
            this._error(err);
            content = parseJSONSafe(cache)
            return _.get(content, "pageItems", []) || [];
            return content;
        }
        throw err;
    }
    await this.snapshot.save(key, content);
    content = parseJSONSafe(content)
    return _.get(content, "pageItems", []) || [];
}

@raphaelsoul
Copy link
Author

另外单元测试 master分支本身就是跑失败

  3) test/client_worker.test.ts
       test features in find address mode
         mock error
           should throw error if http status is 409:

      Uncaught AssertionError [ERR_ASSERTION]:   # client.test.ts:53
  
  assert(err.message === 'mock error')
         |   |       |                
         |   |       false            
         |   "[Nacos#ServerListManager] request url: http://acm.aliyun.com:8080/nacos/serverlist failed with statusCode: 409"
         Error{message:"[Nacos#ServerListManager] request url: http://acm.aliyun.com:8080/nacos/serverlist failed with statusCode: 409"}
  
  --- [string] 'mock error'
  +++ [string] err.message
  @@ -1,10 +1,110 @@
  -mock error
  +[Nacos#ServerListManager] request url: http://acm.aliyun.com:8080/nacos/serverlist failed with statusCode: 409
  
  
      + expected - actual

      -false
      +true
      
      at DataClient.client.on.err (test/client.test.ts:53:7)
      at Immediate.setImmediate (src/client.ts:244:37)
      [use `--full-trace` to display the full stack trace]

@raphaelsoul raphaelsoul changed the title Implement getConfig method for ConfigClient Implement getConfigs method for ConfigClient Apr 21, 2020
@czy88840616
Copy link
Collaborator

这个感觉是个breaking 了,可以单开一个 API,比如 getConfigsByAppName 啥的。

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants