Alex, thank you, I'm convinced now, no objections to ThinClientConfiguration.
On Wed, Aug 28, 2019 at 12:59 PM Alex Plehanov <plehanov.a...@gmail.com> wrote: > Pavel, we already have OdbcEnabled, JdbcEnabled and ThinClientEnabled > properties inside ClientConnectorConfiguration. ThinClientEnabled=false > does not assume that JDBC thin client is disabled, it's clear and not > confusing. I'm not agreed that the same situation with > ThinClientConfiguration will be confusing, the same logic works here as for > ThinClientEnabled flag. At least we can add JavaDoc that this configuration > is not related to JDBC and ODBC, but I think it's redundant. > > ср, 28 авг. 2019 г. в 12:41, Pavel Tupitsyn <ptupit...@apache.org>: > > > ThinClientConfiguration name is very confusing in existing situation. > > E.g. does it apply to JDBC Thin Client? No, it does not, but it is easy > to > > assume it does. > > > > On Tue, Aug 27, 2019 at 5:07 PM Alex Plehanov <plehanov.a...@gmail.com> > > wrote: > > > > > Ilya, Igor, > > > > > > Nested property is what exactly I've done in the last fix. > > > ClientConnectorConfiguration now includes ThinClientConfiguration which > > > contain only one property MaxActiveTxPerConnection for now. > > > > > > Pavel, > > > > > > Why do you think that nested ThinClientConfiguration is more confusing > > than > > > property in common configuration which related only to part of > > configurable > > > elements? In case of nested configuration, user will know that property > > is > > > related only to thin client even without reference to JavaDoc. > > > Now it's only one such property, but if continue to introduce specific > > > properties to the common configuration in the future, after a while > there > > > will be a mess. > > > > > > > > > вт, 27 авг. 2019 г. в 15:18, Ilya Kasnacheev < > ilya.kasnach...@gmail.com > > >: > > > > > > > Hello! > > > > > > > > I think the nested property approach is correct. Sorry for causing > the > > > > confusion. > > > > > > > > Regards, > > > > -- > > > > Ilya Kasnacheev > > > > > > > > > > > > вт, 27 авг. 2019 г. в 15:06, Igor Sapego <isap...@apache.org>: > > > > > > > > > Ilya, > > > > > > > > > > Sorry, I've just got your first message wrong. I though, you were > > > > > proposing to remove ClientConnectorConfiguration altogether, my > bad. > > > > > > > > > > Now, about separating ClientConnectorConfiguration and - I do not > > > > > propose to make it a copy with the same options. What I was > proposing > > > > > is to keep common settings in ClientConnectorConfiguration and > place > > > > > thin client specific things in a separate class which is going to > be > > > > nested > > > > > as a property of ClientConnectorConfiguration. > > > > > > > > > > Best Regards, > > > > > Igor > > > > > > > > > > > > > > > On Tue, Aug 27, 2019 at 12:12 PM Ilya Kasnacheev < > > > > > ilya.kasnach...@gmail.com> > > > > > wrote: > > > > > > > > > > > Hello! > > > > > > > > > > > > I don't see why it should break backward compatibility and > > protocol. > > > > Can > > > > > > you please elaborate? I imagine that Thin client with TX muxing > > > support > > > > > > will just send different requests to which server will respond > > > > > differently. > > > > > > Why would anything break? > > > > > > > > > > > > Regards, > > > > > > -- > > > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > > > > пн, 26 авг. 2019 г. в 14:16, Igor Sapego <isap...@apache.org>: > > > > > > > > > > > > > Ilya, > > > > > > > > > > > > > > This will break backward compatibility and probably protocol, > and > > > > this > > > > > is > > > > > > > not something we should discuss in the context of this specific > > > task. > > > > > > More > > > > > > > like this is a topic for 3.0 wishlist. > > > > > > > > > > > > > > Best Regards, > > > > > > > Igor > > > > > > > > > > > > > > > > > > > > > On Mon, Aug 26, 2019 at 12:28 PM Ilya Kasnacheev < > > > > > > > ilya.kasnach...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > Hello! > > > > > > > > > > > > > > > > Also, let's not add IGNITE_ settings for options that can > > > > reasonably > > > > > be > > > > > > > > configured from IgniteConfiguration. Let's keep it for very > > edge > > > > > cases. > > > > > > > > > > > > > > > > Regards, > > > > > > > > -- > > > > > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > > > > > > > > > > пн, 26 авг. 2019 г. в 12:27, Ilya Kasnacheev < > > > > > > ilya.kasnach...@gmail.com > > > > > > > >: > > > > > > > > > > > > > > > > > Hello! > > > > > > > > > > > > > > > > > > Do we still need to separate client connector configuration > > > from > > > > > thin > > > > > > > > > connector configuration from ODBC connector configuration? > > > > > > > > > > > > > > > > > > I think this is a bad practice: For example, people often > > turn > > > on > > > > > SSL > > > > > > > or > > > > > > > > > auth on just a subset of connectors, think they are secure, > > > when > > > > in > > > > > > > fact > > > > > > > > > they still have unsecured connector around (e.g. ODBC) and > > > their > > > > > data > > > > > > > is > > > > > > > > > not protected at all. > > > > > > > > > > > > > > > > > > It may solve some specific issue that you are facing, but > for > > > > > > newcomers > > > > > > > > to > > > > > > > > > project it is a drawback. I think we should seek to not add > > > > > connector > > > > > > > > > configurations anymore. > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > -- > > > > > > > > > Ilya Kasnacheev > > > > > > > > > > > > > > > > > > > > > > > > > > > пт, 23 авг. 2019 г. в 20:49, Alex Plehanov < > > > > > plehanov.a...@gmail.com > > > > > > >: > > > > > > > > > > > > > > > > > >> Pavel, > > > > > > > > >> > > > > > > > > >> ClientConnectorConfiguration is related to JDBC, ODBC and > > thin > > > > > > > clients, > > > > > > > > >> the > > > > > > > > >> new property only related to thin clients. If we put the > new > > > > > > property > > > > > > > > >> directly into ClientConnectorConfiguration, someone might > > > think > > > > > that > > > > > > > it > > > > > > > > >> also affects JDBC and ODBC. > > > > > > > > >> > > > > > > > > >> пт, 23 авг. 2019 г. в 19:59, Pavel Tupitsyn < > > > > ptupit...@apache.org > > > > > >: > > > > > > > > >> > > > > > > > > >> > Igor, Alex, > > > > > > > > >> > > > > > > > > > >> > Not sure I agree with this: ThinClientConfiguration > inside > > > > > > > > >> > ClientConnectorConfiguration. > > > > > > > > >> > Very confusing IMO, because ClientConnectorConfiguration > > is > > > > > > already > > > > > > > > >> related > > > > > > > > >> > to thin clients only. > > > > > > > > >> > > > > > > > > > >> > Why not put the new property directly into > > > > > > > > ClientConnectorConfiguration? > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
