ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1919

Update the C implementation of removeWatches to have it match ZOOKEEPER-1910

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 3.5.0
    • Fix Version/s: 3.5.2, 3.6.0
    • Component/s: c client
    • Labels:
      None
    1. ZOOKEEPER-1919.patch
      18 kB
      Raul Gutierrez Segales
    2. ZOOKEEPER-1919.patch
      8 kB
      Raul Gutierrez Segales

      Issue Links

        Activity

        Hide
        Patrick Hunt added a comment -

        Michi Mutsuzaki or Camille Fournier could you look at this follow-on to ZOOKEEPER-1910?

        Show
        Patrick Hunt added a comment - Michi Mutsuzaki or Camille Fournier could you look at this follow-on to ZOOKEEPER-1910 ?
        Hide
        Camille Fournier added a comment -

        Unfortunately my C is rusty to put it mildly. Michi Mutsuzaki?

        Show
        Camille Fournier added a comment - Unfortunately my C is rusty to put it mildly. Michi Mutsuzaki ?
        Hide
        Raul Gutierrez Segales added a comment -

        Oops, sorry for the delay. I'll come up with the patch in the next days. Thanks for the follow-up Patrick Hunt!

        Show
        Raul Gutierrez Segales added a comment - Oops, sorry for the delay. I'll come up with the patch in the next days. Thanks for the follow-up Patrick Hunt !
        Hide
        Raul Gutierrez Segales added a comment -

        Sorry for the delay here. This patch updates the C implementation so that the remove watches API is vis-à-vis with the Java implementation, according to Rakesh R's latest changes.

        I'll add some tests tomorrow, but wanted to put this out to get feedback on the how the API will look (no big changes though).

        It would be nice to have this in before the first 3.5.0 alpha release, to avoid having people start using the differing APIs (between Java and C).

        cc: Patrick Hunt, Flavio Junqueira, Michi Mutsuzaki

        Show
        Raul Gutierrez Segales added a comment - Sorry for the delay here. This patch updates the C implementation so that the remove watches API is vis-à-vis with the Java implementation, according to Rakesh R 's latest changes. I'll add some tests tomorrow, but wanted to put this out to get feedback on the how the API will look (no big changes though). It would be nice to have this in before the first 3.5.0 alpha release, to avoid having people start using the differing APIs (between Java and C). cc: Patrick Hunt , Flavio Junqueira , Michi Mutsuzaki
        Hide
        Rakesh R added a comment -

        Thanks Raul Gutierrez Segales for the patch. Just few minor comments:

        1. Instead of "watchers", can we use "watches". Just to sync with the java side API names.
        2. It would be good to rename union - 'RemoveWatches' to 'WatchesRequest' or a better name, as this union has two types of requests, one for just checking the watch existence and another for remove watch.
        Show
        Rakesh R added a comment - Thanks Raul Gutierrez Segales for the patch. Just few minor comments: Instead of "watchers", can we use "watches". Just to sync with the java side API names. It would be good to rename union - 'RemoveWatches' to 'WatchesRequest' or a better name, as this union has two types of requests, one for just checking the watch existence and another for remove watch.
        Hide
        Raul Gutierrez Segales added a comment -

        Sorry for the delay Rakesh R! I've addressed your comments plus I cleaned up some related items too (i.e.: consistent naming, better tests, etc.).

        cc: Patrick Hunt, hopefully a thing for rc2.

        Show
        Raul Gutierrez Segales added a comment - Sorry for the delay Rakesh R ! I've addressed your comments plus I cleaned up some related items too (i.e.: consistent naming, better tests, etc.). cc: Patrick Hunt , hopefully a thing for rc2.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12659496/ZOOKEEPER-1919.patch
        against trunk revision 1615240.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659496/ZOOKEEPER-1919.patch against trunk revision 1615240. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2261//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12659496/ZOOKEEPER-1919.patch
        against trunk revision 1672934.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659496/ZOOKEEPER-1919.patch against trunk revision 1672934. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2615//console This message is automatically generated.

          People

          • Assignee:
            Raul Gutierrez Segales
            Reporter:
            Raul Gutierrez Segales
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development