Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

add createClient API with clientOptions #49

Merged

Conversation

foreverneverer
Copy link
Contributor

@foreverneverer foreverneverer commented May 9, 2019

添加了createClient(clientOptions) 、getSingletonClient(clientOptions)接口,使得用户可以直接传入配置参数(而不用写配置文件)就可以创建client

该功能为小米大数据临时需求。

@vagetablechicken
Copy link
Member

只提供meta_server和timeout么?其他的config,例如workers,counter相关的参数呢?


public class ClientOptions {
String metaServers = "127.0.0.1:34601,127.0.0.1:34602,127.0.0.1:34603";
int timeout = 1000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为了和operation_timeout 保持一致,用operationTimeout更好

package com.xiaomi.infra.pegasus.client;

public class ClientOptions {
String metaServers = "127.0.0.1:34601,127.0.0.1:34602,127.0.0.1:34603";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还有几个options都加上吧:

  • async_workers
  • enable_perf_counter
  • perf_counter_tags
  • push_counter_interval_secs

@neverchanje
Copy link

另外配置项需要是 private final 的,构造的时候用 builder 模式,我建议你参照 https://github.com/lettuce-io/lettuce-core/blob/master/src/main/java/io/lettuce/core/ClientOptions.java 来重写。

@foreverneverer
Copy link
Contributor Author

已经修改,并使用builder模式重构 @qinzuoyan @neverchanje @hycdong @vagetablechicken

import java.time.Duration;

/**
* @author jiashuo1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

你要想进 apache 的话是不能加 author 的

qinzuoyan
qinzuoyan previously approved these changes May 27, 2019
@neverchanje neverchanje merged commit 4f8bd7d into XiaoMi:thrift-0.11.0-inlined May 27, 2019
neverchanje pushed a commit that referenced this pull request Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants