Re: Name resolution in StaticHostProvider

Flavio, did you have a chance to think about whether we should use unchecked exception here? Andor On Sat, May 12, 2018 at 1:02 PM, Norbert Kalmar wrote: > Hi, > > In my opinion, when the client calls next(), and sees that this call could > return with an

Re: Name resolution in StaticHostProvider

Hi, In my opinion, when the client calls next(), and sees that this call could return with an UnknownHostException, it will assume next() is giving back the next available address in the list, whether it's a valid address or not. If next() would throw something like

Re: Name resolution in StaticHostProvider

Hi team, Anyone else has thoughts about whether we should use checked or unchecked exception here? Thanks, Andor On Thu, May 10, 2018 at 8:19 AM, Andor Molnar wrote: > Interesting idea. What difference you think it could make comparing to > checked? > > > > On Wed, May

Re: Name resolution in StaticHostProvider

Interesting idea. What difference you think it could make comparing to checked? On Wed, May 9, 2018 at 8:01 AM, Flavio Junqueira wrote: > I'm actually now wondering whether we should be using an unchecked > exception instead. A lot of things have changed with exception

Re: Name resolution in StaticHostProvider

I'm actually now wondering whether we should be using an unchecked exception instead. A lot of things have changed with exception handling since we wrote this code base initially. An unchecked exception would actually match better my current mental model of what that signature should look like.

Re: Name resolution in StaticHostProvider

I like the idea of indicating to the application that there is something wrong with the list of servers so that it has a chance to look into it. With the current code in `ClientCnxn`, we will log at warn level and hope that someone sees it, but we are not really stopping the client. Throwing

Re: Name resolution in StaticHostProvider

Let's put it this way: 1- The interpretation of next() is "give me the next server's IP address that I should connect to and do whatever preparation (resolution) is necessary to do so". The answer could be: a) I'm done, here it is, b) there's a problem with the next item in the list, for instance

Re: Name resolution in StaticHostProvider

The refactoring did not seem justifiable at first, so my reaction to it. You have clarified the reason for including the changes, and I actually like it. About the exception, there are two points for me: 1- You don't really need to throw to execute the plan you described. 2- In the case we do

Re: Name resolution in StaticHostProvider

Sorry, I thought you were against the whole refactoring. "2- That we discuss separately whether we want to change the behaviour of the next()in the HostProvider interface." >From this it seemed to me, it's not just a polishing issue, but maybe I've gotten you wrong. Anyway, there're 2 contention

Re: Name resolution in StaticHostProvider

I'm happy to do that once we have an agreement. On Tue, May 8, 2018 at 8:34 AM, Flavio Junqueira wrote: > It might be a good idea to document whatever we end up doing. > > -Flavio > > > On 8 May 2018, at 17:22, Andor Molnar wrote: > > > > "If refactoring

Re: Name resolution in StaticHostProvider

Can you list what the contention points are according to you? Feel free to do that in the PR as well, I want to make sure we have the same understanding of the points that still need to be resolved. From where I stand, there is no major issue pending other than one polishing issue that I

Re: Name resolution in StaticHostProvider

It might be a good idea to document whatever we end up doing. -Flavio > On 8 May 2018, at 17:22, Andor Molnar wrote: > > "If refactoring is necessary to address the issue, then it should be part > of the PR." > > Agreed. I think it is and it also makes things a lot more

Re: Name resolution in StaticHostProvider

"If refactoring is necessary to address the issue, then it should be part of the PR." Agreed. I think it is and it also makes things a lot more simpler, so it's easier to review. "I'm not sure what kind of confirmation you are after here. Could you elaborate, please?" I'm just wondering what

Re: Name resolution in StaticHostProvider

Hi Andor, Thanks for your work on addressing the issue. > On 8 May 2018, at 16:06, Andor Molnar wrote: > > Hi, > > Updating this thread, because the PR is still being review on GitHub. > > So, the reason why I refactored the original behaviour of > StaticHostProvider is

Re: Name resolution in StaticHostProvider

Hi, Updating this thread, because the PR is still being review on GitHub. So, the reason why I refactored the original behaviour of StaticHostProvider is that I believe that it's trying to do something which is not its responsibility. Please tell me if there's a good historical reason for that.

Re: Name resolution in StaticHostProvider

Hi Abe, Unfortunately we haven't got any feedback yet. What do you think of implementing Option #3? Regards, Andor On Thu, Feb 22, 2018 at 6:06 PM, Andor Molnar wrote: > Did anybody happen to take a quick look by any chance? > > I don't want to push this too hard, because

Re: Name resolution in StaticHostProvider

Did anybody happen to take a quick look by any chance? I don't want to push this too hard, because I know it's a time consuming topic to think about, but this is a blocker in 3.5 which has been hanging around for a while and any feedback would be extremely helpful to close it quickly. Thanks,