You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@solr.apache.org by Alex Deparvu <st...@apache.org> on 2023/03/15 18:13:21 UTC
Alias property deletion confusion
Hi,
I am working on v2 apis for managing alias properties as part of SOLR-16393.
I have a confusion I would love to clarify with the community related to an
existing test (AliasIntegrationTest) that deletes an alias property.
My understanding is that setting a property to 'empty string' will
effectively remove it. So what happens when a property is set to null?
What is the expectation for the following? The test currently lacks any
assertions [0].
var setAliasProperty =
CollectionAdminRequest.setAliasProperty(aliasName);
setAliasProperty.addProperty("bar", null);
My feeling was that setting it to null will remove it, but that does not
seem to be the case [1], it seems to not change it at all.
best,
alex
[0]
https://github.com/apache/solr/blob/a2a39fb136a9338f4603748bf446038cf4155296/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java#L301
[1]
https://github.com/apache/solr/pull/1459/files#diff-e4ea077299d4865546c5a8bdf8ba618d1dbb06fc01bf95eeacde1231ff11fcd7R331
Re: Alias property deletion confusion
Posted by Alex Deparvu <st...@apache.org>.
Thank you Gus and David for the very prompt replies!
I was planning on updating the test with some assertions (that is how I ran
into this).
It seems that today setting to null will be a no-op, the property will not
be changed. I'm not 100% sure what component handles this, it may be Zk.
In my PR I have changed the 'set null value' behavior to be consistent with
the 'set empty value' and updated all tests with assertions.
If anyone disagrees please let me know here or on the PR itself [0].
best,
alex
[0] https://github.com/apache/solr/pull/1459
On Wed, Mar 15, 2023 at 3:01 PM Gus Heck <gu...@gmail.com> wrote:
> Yeah it does look like that test should be improved. I would expect that
> null and "" should be treated equivalently, but I don't recall what the
> code actually does. We should get some asserts in there and decide if we
> like the results of what we find. If not, fix it.
>
> On Wed, Mar 15, 2023 at 4:49 PM David Smiley <ds...@apache.org> wrote:
>
> > Interestingly, the part of that test which you commented on doesn't
> appear
> > to assert/validate anything; it just does stuff.
> > I suspect Gus may know the answer to your question.
> >
> > ~ David Smiley
> > Apache Lucene/Solr Search Developer
> > http://www.linkedin.com/in/davidwsmiley
> >
> >
> > On Wed, Mar 15, 2023 at 2:13 PM Alex Deparvu <st...@apache.org>
> wrote:
> >
> > > Hi,
> > >
> > > I am working on v2 apis for managing alias properties as part of
> > > SOLR-16393.
> > >
> > > I have a confusion I would love to clarify with the community related
> to
> > an
> > > existing test (AliasIntegrationTest) that deletes an alias property.
> > > My understanding is that setting a property to 'empty string' will
> > > effectively remove it. So what happens when a property is set to null?
> > >
> > > What is the expectation for the following? The test currently lacks any
> > > assertions [0].
> > > var setAliasProperty =
> > > CollectionAdminRequest.setAliasProperty(aliasName);
> > > setAliasProperty.addProperty("bar", null);
> > >
> > > My feeling was that setting it to null will remove it, but that does
> not
> > > seem to be the case [1], it seems to not change it at all.
> > >
> > > best,
> > > alex
> > >
> > > [0]
> > >
> > >
> >
> https://github.com/apache/solr/blob/a2a39fb136a9338f4603748bf446038cf4155296/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java#L301
> > >
> > > [1]
> > >
> > >
> >
> https://github.com/apache/solr/pull/1459/files#diff-e4ea077299d4865546c5a8bdf8ba618d1dbb06fc01bf95eeacde1231ff11fcd7R331
> > >
> >
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)
>
Re: Alias property deletion confusion
Posted by Gus Heck <gu...@gmail.com>.
Yeah it does look like that test should be improved. I would expect that
null and "" should be treated equivalently, but I don't recall what the
code actually does. We should get some asserts in there and decide if we
like the results of what we find. If not, fix it.
On Wed, Mar 15, 2023 at 4:49 PM David Smiley <ds...@apache.org> wrote:
> Interestingly, the part of that test which you commented on doesn't appear
> to assert/validate anything; it just does stuff.
> I suspect Gus may know the answer to your question.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>
>
> On Wed, Mar 15, 2023 at 2:13 PM Alex Deparvu <st...@apache.org> wrote:
>
> > Hi,
> >
> > I am working on v2 apis for managing alias properties as part of
> > SOLR-16393.
> >
> > I have a confusion I would love to clarify with the community related to
> an
> > existing test (AliasIntegrationTest) that deletes an alias property.
> > My understanding is that setting a property to 'empty string' will
> > effectively remove it. So what happens when a property is set to null?
> >
> > What is the expectation for the following? The test currently lacks any
> > assertions [0].
> > var setAliasProperty =
> > CollectionAdminRequest.setAliasProperty(aliasName);
> > setAliasProperty.addProperty("bar", null);
> >
> > My feeling was that setting it to null will remove it, but that does not
> > seem to be the case [1], it seems to not change it at all.
> >
> > best,
> > alex
> >
> > [0]
> >
> >
> https://github.com/apache/solr/blob/a2a39fb136a9338f4603748bf446038cf4155296/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java#L301
> >
> > [1]
> >
> >
> https://github.com/apache/solr/pull/1459/files#diff-e4ea077299d4865546c5a8bdf8ba618d1dbb06fc01bf95eeacde1231ff11fcd7R331
> >
>
--
http://www.needhamsoftware.com (work)
http://www.the111shift.com (play)
Re: Alias property deletion confusion
Posted by David Smiley <ds...@apache.org>.
Interestingly, the part of that test which you commented on doesn't appear
to assert/validate anything; it just does stuff.
I suspect Gus may know the answer to your question.
~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley
On Wed, Mar 15, 2023 at 2:13 PM Alex Deparvu <st...@apache.org> wrote:
> Hi,
>
> I am working on v2 apis for managing alias properties as part of
> SOLR-16393.
>
> I have a confusion I would love to clarify with the community related to an
> existing test (AliasIntegrationTest) that deletes an alias property.
> My understanding is that setting a property to 'empty string' will
> effectively remove it. So what happens when a property is set to null?
>
> What is the expectation for the following? The test currently lacks any
> assertions [0].
> var setAliasProperty =
> CollectionAdminRequest.setAliasProperty(aliasName);
> setAliasProperty.addProperty("bar", null);
>
> My feeling was that setting it to null will remove it, but that does not
> seem to be the case [1], it seems to not change it at all.
>
> best,
> alex
>
> [0]
>
> https://github.com/apache/solr/blob/a2a39fb136a9338f4603748bf446038cf4155296/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java#L301
>
> [1]
>
> https://github.com/apache/solr/pull/1459/files#diff-e4ea077299d4865546c5a8bdf8ba618d1dbb06fc01bf95eeacde1231ff11fcd7R331
>