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? > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
