search

uber / tchannel

network multiplexing and framing protocol for RPC
MIT License
1.15k stars 129 forks source link

Improve hyperbahn client host port list parsing #1279

Closed anson627 closed 9 years ago

anson627 commented 9 years ago

To fix #1275 Host port file reading and parsing error need be exposed to users.

@Willyham @ShanniLi @Raynos

ShanniLi commented 9 years ago

I think @Willyham meant to expose the file read error, which is different from this PR. Not sure merging hostPortFile into hostPortList is a good idea. hostPortList by its name is a list/array

anson627 commented 9 years ago

Exposing error is part of this PR. The other part is to simplify the interface, having two parameter for the same thing make it hard to use. For example, xlate has hostPortFile in config file, if I want to overwrite it by passing --config hostPortFile [localhost:21300]. I need write code like this: if (Array.isArray(hostPortFile)), call constructor with hostPortList, otherwise, call with hostPortFile.

Raynos commented 9 years ago

:-1: This is confusing.

Let's have type stability and two arguments.

Raynos commented 9 years ago

Please split this into two commits or two PRs.

The improved error messages are :+1:

anson627 commented 9 years ago

Will do the split.

ShanniLi commented 9 years ago

one comment. Otherwise, lgtm.