I disagree.
I think /setProxy(ProxyConfiguration)/ is 1st prize.
If we are concerned that users will not know WHAT options are
available.. We could always have a static builder for our supported
options.
--Udo
On 3/10/20 10:07 AM, Dan Smith wrote:
Ok, how about this?
setProxy(SniProxyConfiguration config)
interface SniProxyConfiguration extends ProxyConfiguration {
static SniProxyConfiguration create(String host, int port);
String getHost();
int getPort()
}
The main difference here from John's proposal being that setProxy takes a
SniProxyConfiguration, rather than the base ProxyConfiguration. We can
overload setProxy later if needed.
The reason I'm not as keen on setProxy(ProxyConfiguration) is that it is
hard for the user to discover the different types of ProxyConfiguration
subclasses and know what is supported.
-Dan
On Mon, Mar 9, 2020 at 11:23 PM John Blum <jb...@pivotal.io> wrote:
Corrections ( :-P ), my apologies...
Iterable<ProxyConfiguration> proxyConfigurations = ...;
StreamSupport.stream(proxyConfiguration*s*.*spliterator*(), false)
.filter(config -> config.getType.isSecure()) *// This could be
improved; see below...*
.*forEach*(config -> // do something with *each* secure proxy
*config*).
Although, I am sure you got the point, ;-)
With improvement:
interface ProxyConfiguration {
ProxyType getType();
// null-safe!
default boolean isSecure() {
ProxyType type = getType();
return type != null && type.isSecure();
}
}
Then:
StreamSupport.stream(..)
* .filter(ProxyConfiguration::isSecure)*
.forEach(...);
Again, completely contrived.
Cheers!
-j
On Mon, Mar 9, 2020 at 11:14 PM John Blum <jb...@pivotal.io> wrote:
Yes, it's redundant (i.e. Enum + class type).
However, having an Enum in addition to a specific type (1 reason I
defaulted the getType() method) can still be useful, such as in a switch
statement for example. Enums are, well, easier to enumerate (useful in
Streams with filters). Maybe you are going to classify certain Proxy's
by Enum type, for example:
enum ProxyType {
ONE,
TWO,
THREE;
// Contrived example
public boolean isSecure() {
return Arrays.asList(ONE, THREE).contains(this);
}
}
Then:
Iterable<ProxyConfiguration> proxyConfigurations = ...;
StreamSupport.stream(proxyConfiguration.spilterator(), false)
.filter(config -> config.getType.isSecure())
.ifPresent(config -> // do something with secure proxy).
Food for thought.
-j
On Mon, Mar 9, 2020 at 7:11 PM Jacob Barrett <jbarr...@pivotal.io>
wrote:
Yes it’s redundant.
On Mar 9, 2020, at 5:08 PM, Bill Burcham <bill.burc...@gmail.com>
wrote:
What I like about John's full-fledged-class-per-proxy-kind is that
everything that can potentially vary with proxy kind is all together
in
a
single object.
That being said, John, in your SniProxyConfiguration, it seems to me
that
the class itself (SniProxyConfiguration) could easily stand for the
proxy
"type". If that's true then we could view ProxyType as redundant and
simply
eliminate it right?
On Mon, Mar 9, 2020 at 2:41 PM Jacob Barrett <jbarr...@pivotal.io>
wrote:
+1 to Anthony and John. See similar comments in the RFC.
On Mar 9, 2020, at 12:08 PM, Anthony Baker <aba...@pivotal.io>
wrote:
I’m not suggesting encoding the the proxy type in the URI. Just
wondering if we can support stronger typing than String for defining
host/port/url configuration. As John notes, later in the thread,
perhaps
using a configuration interface may help.
Anthony
On Mar 9, 2020, at 11:11 AM, Bill Burcham <bill.burc...@gmail.com>
wrote:
Anthony and Jacob, I can see how the proposed ProxyType parameter
could
fit
into the scheme part of a a URI. However, the problem that
introduces is
that we would then have to pick (named) URL schemes to support. But
URL
schemes are standardized and it's not obvious which of the standard
ones
might apply here.
If we couldn't adopt a standard scheme, we'd have to make one up.
At
that
point I question the value of putting the (made-up) scheme into a
URI
string.
For this reason, I am a fan of the ProxyType parameter over a
made-up
URL
scheme.
That leaves open Anthony's other idea: eliminating the ProxyType
parameter
in favor of a separate method to set each kind of proxy. In the
current
RFC, that's just one, e.g. setPoolProxyWithSNI. I guess that comes
down
to:
what's the likelihood of us supporting other proxy types soon, and
then
what's the value of having a single method (and multiple enums)
versus
multiple methods (and no enum). If we thought the proxyAddress
parameter
would carry different information across proxy types that might
tilt
us
toward the separate methods. The two on the table, however, (SNI,
SOCKS5)
both have identical proxyAddress information.
On Mon, Mar 9, 2020 at 10:54 AM Bill Burcham <
bill.burc...@gmail.com
wrote:
By popular demand we are extending the RFC review period. I know
Udo
asked
for Friday (and Joris +1'd it), but since this is a small RFC,
we'd
like to
try to close it by Wednesday, March 11, ok?
On Mon, Mar 9, 2020 at 10:39 AM Jacob Barrett <
jbarr...@pivotal.io>
wrote:
I raised similar concerns as a comment in the RFC.
On Mar 9, 2020, at 10:29 AM, Anthony Baker <aba...@pivotal.io>
wrote:
Given this new API:
setPoolProxy(ProxyType type, String proxyAddress)
The ProxyType enum seems to be a look ahead at supporting other
kinds
of proxies. What is your thinking about using the enum vs
specific
named
API’s (e.g. setPoolProxyWithSNI).
Currently the definition of the proxyAddress seems to be
dependent of
the proxy type. Did you consider stronger typing using an URI
parameter
type?
Anthony
On Mar 6, 2020, at 11:04 AM, Bill Burcham <
bill.burc...@gmail.com>
wrote:
Please review the RFC for *Client side configuration for a SNI
proxy*:
https://cwiki.apache.org/confluence/display/GEODE/Client+side+configuration+for+a+SNI+proxy
Please comment by Monday, March 9, 2020.
Regards,
Bill and Ernie
--
-John
Spring Data Team
--
-John
Spring Data Team
