geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Huynh <jhu...@pivotal.io>
Subject Re: [DISCUSS] changes to registerInterest API
Date Fri, 17 Nov 2017 21:24:07 GMT
Thanks John for the clarification!

On Fri, Nov 17, 2017 at 1:12 PM John Blum <jblum@pivotal.io> wrote:

> This...
>
> > The Iterable version would handle any collection type by having the user
> pass
> in the iterator for the collection.
>
> Is not correct.
>
> The Collection<E> interface itself "extends" the java.lang.Iterable<E>
> interface (see here...
> https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html under
> "*All
> Superinterfaces*").
>
> Therefore a user can simply to this...
>
> *List*<KeyType> keys = ...
>
> region.registerInterest(keys); *// calls the
> Region.registerInterest(:Iterable<T>) method.*
>
> Alternatively, this would also be allowed...
>
> *Set*<KeyType> keys = ...
>
> region.registerInterest(keys);
>
>
> On Fri, Nov 17, 2017 at 11:44 AM, Jason Huynh <jhuynh@pivotal.io> wrote:
>
> > Current idea is to:
> > - deprecate current "ALL_KEYS" and List passing behavior in
> > registerInterest()
> > - add registerInterestAllKeys();
> > - add registerInterest(T... keys) and registerInterest(Iterable<T>keys)
> > and
> > not have one specifically for List or specific collections.
> >
> > The Iterable version would handle any collection type by having the user
> > pass in the iterator for the collection.
> >
> > On Fri, Nov 17, 2017 at 11:32 AM Jacob Barrett <jbarrett@pivotal.io>
> > wrote:
> >
> > > I am failing to see where registerInterest(List<T> keys) is an issue
> for
> > > the key type in the region. If our region is Region<String> then I
> would
> > > expect registerInterest(List<String>). If the keys are unknown or a mix
> > > then you should have Region<Object> and thus
> > registerInterest(List<Object).
> > >
> > > I echo John's statements on VarArgs and type erasure as well as his
> > > argument for Iterable<T>.
> > >
> > > Also, List<T> does not restrict you from List indexes. The region would
> > be
> > > Region<List<String>> with registerInterest<List<List<String>>().
> > >
> > > -Jake
> > >
> > >
> > > On Fri, Nov 17, 2017 at 10:04 AM John Blum <jblum@pivotal.io> wrote:
> > >
> > > > Personally, I prefer the var args method (registerInterest(T...
> keys))
> > > > myself.  It is way more convenient if I only have a few keys when
> > calling
> > > > this method then to have to add the keys to a List, especially for
> > > testing
> > > > purposes.
> > > >
> > > > But, I typically like to pair that with a
> registerInterest(Iterable<T>
> > > > keys) method
> > > > as well.  By having a overloaded Iterable variant, then I can pass in
> > any
> > > > Collection type I want (which shouldn't be restricted to just List).
> > It
> > > > also is a simple matter to convert any *Collection* (i.e. *List*,
> > *Set*,
> > > > etc) to an array, which can be passed to the var args method.  By
> using
> > > > List,
> > > > you are implying that "order matters" since a List is a order
> > collection
> > > of
> > > > elements.
> > > >
> > > > This ("*It might even cause problems of pushing in **multiple
> different
> > > > types.*"), regarding var args, does not even make sense. Technically,
> > > > List<T> is no different.  Java's type erasure essentially equates
var
> > > args
> > > > too "Object..." (or Object[]) and the List<T> to List (or a List
of
> > > > Objects,
> > > > essentially like if you just did this... List<Object>) So, while
the
> > > > compiler ensures compile-time type-safety of generics, there is no
> > > generics
> > > > type-safety guarantees at runtime.
> > > >
> > > >
> > > >
> > > > On Fri, Nov 17, 2017 at 9:22 AM, Jason Huynh <jhuynh@pivotal.io>
> > wrote:
> > > >
> > > > > Hi Mike,
> > > > >
> > > > > The current support for List leads to compilation issues if the
> > region
> > > is
> > > > > type constrained.  However I think you are suggesting instead of
a
> > var
> > > > args
> > > > > method, instead provide a registerInterest(List keys) method?
> > > > >
> > > > > So far what I am hearing requested is:
> > > > > deprecate current "ALL_KEYS" and List passing behavior
> > > > > registerInterestAllKeys();
> > > > > registerInterest(List<T> keys) instead of a registerInterest(T...
> > keys)
> > > > >
> > > > > Will anyone ever actually have a List as the key itself? The
> current
> > > and
> > > > > suggested changes would not allow it registering for a specific
> List
> > > > > object.
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Nov 16, 2017 at 6:50 PM Jacob Barrett <jbarrett@pivotal.io
> >
> > > > wrote:
> > > > >
> > > > > > Geode Native C++ and .NET have:
> > > > > >
> > > > > >   virtual void registerKeys(const
> > > > > > std::vector<std::shared_ptr<CacheableKey>> &
keys,
> > > > > >                             bool isDurable = false,
> > > > > >                             bool getInitialValues = false,
> > > > > >                             bool receiveValues = true) = 0;
> > > > > >
> > > > > >   virtual void unregisterKeys(const
> > > > > > std::vector<std::shared_ptr<CacheableKey>> &
keys) = 0;
> > > > > >
> > > > > >   virtual void *registerAllKeys*(bool isDurable = false,
> > > > > >                                bool getInitialValues = false,
> > > > > >                                bool receiveValues = true) =
0;
> > > > > >
> > > > > >   virtual void unregisterAllKeys() = 0;
> > > > > >
> > > > > >   virtual void registerRegex(const std::string& regex,
> > > > > >                              bool isDurable = false,
> > > > > >                              bool getInitialValues = false,
> > > > > >                              bool receiveValues = true) = 0;
> > > > > >
> > > > > >   virtual void unregisterRegex(const char* regex) = 0;
> > > > > >
> > > > > > I dislike special values like this so yes please make it go
away!
> > > > > >
> > > > > > -Jake
> > > > > >
> > > > > >
> > > > > > On Thu, Nov 16, 2017 at 5:20 PM Dan Smith <dsmith@pivotal.io>
> > wrote:
> > > > > >
> > > > > > > I don't really like the regex option - it implies that
your
> keys
> > > are
> > > > > all
> > > > > > > strings. Will any other regular expressions work on non
string
> > > > objects?
> > > > > > > registerInterestAllKeys() seems like a better option.
> > > > > > >
> > > > > > > -Dan
> > > > > > >
> > > > > > > On Thu, Nov 16, 2017 at 4:34 PM, Michael Stolz <
> > mstolz@pivotal.io>
> > > > > > wrote:
> > > > > > >
> > > > > > > > I don't like the vararg option.
> > > > > > > > If i'm maintaining a list of keys i'm interested in,
I want
> to
> > be
> > > > > able
> > > > > > to
> > > > > > > > pass that List in.
> > > > > > > > Varargs is a poor substitute. It might even cause
problems of
> > > > pushing
> > > > > > in
> > > > > > > > multiple different types. Keys must all be of one
type for a
> > > given
> > > > > > > Region.
> > > > > > > >
> > > > > > > >
> > > > > > > > I'm very much in favor of deprecating the ALL_KEYS
string in
> > > favor
> > > > of
> > > > > > > > something that is typed specially if you refer to
ALL_KEYS.
> > > > > > > >
> > > > > > > >
> > > > > > > > If that works, then we don't necessarily need the
additional
> > API
> > > > > > > > registerInterestAllKeys(). But if ALL_KEYS can't be
a special
> > > type
> > > > to
> > > > > > get
> > > > > > > > over the compilation issues then we should go with
the new
> API.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Mike Stolz
> > > > > > > > Principal Engineer, GemFire Product Lead
> > > > > > > > Mobile: +1-631-835-4771 <(631)%20835-4771>
> <(631)%20835-4771> <(631)%20835-4771>
> > > <(631)%20835-4771>
> > > > <(631)%20835-4771>
> > > > > > > >
> > > > > > > > On Thu, Nov 16, 2017 at 7:02 PM, Anilkumar Gingade
<
> > > > > > agingade@pivotal.io>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +1 Deprecating ALL_KEYS option; I believe this
is added
> > before
> > > we
> > > > > > > > supported
> > > > > > > > > regex support.
> > > > > > > > >
> > > > > > > > >  Doesn't seems like a new API is needed. The
regex java doc
> > > > clearly
> > > > > > > > > specifies the effect of ".*".
> > > > > > > > >
> > > > > > > > > +1 for deprecating list argument; and replacing
with new
> API.
> > > > > > > > >
> > > > > > > > > -Anil.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > On Thu, Nov 16, 2017 at 3:36 PM, Jason Huynh
<
> > > jhuynh@pivotal.io>
> > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > For GEODE-3813 <
> > > > https://issues.apache.org/jira/browse/GEODE-3813
> > > > > >:
> > > > > > > > > Region
> > > > > > > > > > registerInterest API usage of type parameters
is broken
> > > > > > > > > > <https://issues.apache.org/jira/browse/GEODE-3813>
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The current API to registerInterest allows
a special
> string
> > > > token
> > > > > > > > > > “ALL_KEYS” to be passed in as the parameter
to
> > > > registerInterest(T
> > > > > > > key).
> > > > > > > > > > This special token causes the registerInterest
to behave
> > > > similar
> > > > > to
> > > > > > > > > > registerInterestRegex(“.*”).  As the
ticket states, if
> the
> > > > region
> > > > > > has
> > > > > > > > > been
> > > > > > > > > > typed to anything other than Object or String,
the usage
> of
> > > > > > > “ALL_KEYS”
> > > > > > > > > as a
> > > > > > > > > > parameter results in a compilation error.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Proposals:
> > > > > > > > > >
> > > > > > > > > > I would like to deprecate the special string
“ALL_KEYS”
> and
> > > > > > document
> > > > > > > a
> > > > > > > > > > workaround of using registerInterestRegex(“.*”)
or we can
> > > add a
> > > > > new
> > > > > > > API
> > > > > > > > > > called registerInterestAllKeys()
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I think we should also deprecate passing
a List Object of
> > > keys
> > > > > into
> > > > > > > > > > registerInterest.  It has the same compilation
> restrictions
> > > as
> > > > > > > > “ALL_KEYS”
> > > > > > > > > > when the region is key constrained/typed.
 The reason why
> > > List
> > > > > > would
> > > > > > > be
> > > > > > > > > > used is to allow registering multiple keys
at once.
> > Instead,
> > > > we
> > > > > > can
> > > > > > > > add
> > > > > > > > > a
> > > > > > > > > > new var arg API like registerInterest(T…
keys).  This
> > problem
> > > > and
> > > > > > > > > solution
> > > > > > > > > > was also documented in the ticket by the
ticket creator
> > (Kirk
> > > > > Lund)
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > >
> > > > > > > > > > -Jason
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > -John
> > > > john.blum10101 (skype)
> > > >
> > >
> >
>
>
>
> --
> -John
> john.blum10101 (skype)
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message