ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-732

Improper translation of error into Python exception

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3.0
    • Fix Version/s: 3.4.6, 3.5.0
    • Component/s: contrib-bindings
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Client that uses python binding may receive SystemError on session expiration.

      Description

      Apparently errors returned by the C library are not being correctly converted into a Python exception in some cases:

      >>> zookeeper.get_children(0, "/", None)
      Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      SystemError: error return without exception set

      1. ZOOKEEPER-732-trunk.patch
        1.0 kB
        Flavio Junqueira
      2. ZOOKEEPER-732-b3.4.patch
        1.0 kB
        Flavio Junqueira
      3. ZOOKEEPER-732.patch
        0.8 kB
        Andrei Savu
      4. ZOOKEEPER-732.patch
        0.5 kB
        Lei Zhang

        Activity

        Hide
        Lei Zhang added a comment -

        Seems fixed in 3.3.1:

        >>> zookeeper.get_children(0, "/", None)
        Traceback (most recent call last):
        File "<stdin>", line 1, in ?
        zookeeper.ZooKeeperException: zhandle out of range

        Issue can be closed.

        Show
        Lei Zhang added a comment - Seems fixed in 3.3.1: >>> zookeeper.get_children(0, "/", None) Traceback (most recent call last): File "<stdin>", line 1, in ? zookeeper.ZooKeeperException: zhandle out of range Issue can be closed.
        Hide
        Lei Zhang added a comment -

        Attached is a patch that fixes issue 732. Can somebody please review?

        Show
        Lei Zhang added a comment - Attached is a patch that fixes issue 732. Can somebody please review?
        Hide
        Andrei Savu added a comment -

        I've fixed the patch to also include ZCLOSING and ZNOTHING. It seems like all the error codes listed in zookeeper.h ZOO_ERRORS enum are now handled by err_to_exception.

        Show
        Andrei Savu added a comment - I've fixed the patch to also include ZCLOSING and ZNOTHING . It seems like all the error codes listed in zookeeper.h ZOO_ERRORS enum are now handled by err_to_exception .
        Hide
        Lei Zhang added a comment -

        Thanks Andrei. Patch looks good to me.

        Show
        Lei Zhang added a comment - Thanks Andrei. Patch looks good to me.
        Hide
        Andrii Shyshkalov added a comment -

        This issue isn't closed, and indeed 3.3.3-3.3.5, 3.4.* releases do not contain this fix. What has to be done for it to be committed?

        Show
        Andrii Shyshkalov added a comment - This issue isn't closed, and indeed 3.3.3-3.3.5, 3.4.* releases do not contain this fix. What has to be done for it to be committed?
        Hide
        Michi Mutsuzaki added a comment -

        Sorry Andrii, I don't know how this got closed without getting checked in. I'm reopening the ticket.

        Show
        Michi Mutsuzaki added a comment - Sorry Andrii, I don't know how this got closed without getting checked in. I'm reopening the ticket.
        Hide
        Michi Mutsuzaki added a comment -

        The patch looks good to me, but I like to get +1 from another committer before I check this in since I'm not familiar with zkpython.

        Thanks1
        --Michi

        Show
        Michi Mutsuzaki added a comment - The patch looks good to me, but I like to get +1 from another committer before I check this in since I'm not familiar with zkpython. Thanks1 --Michi
        Hide
        Flavio Junqueira added a comment -

        The fix looks reasonable to me, but I'm also not that familiar to tell.

        Show
        Flavio Junqueira added a comment - The fix looks reasonable to me, but I'm also not that familiar to tell.
        Hide
        Nikita Vetoshkin added a comment -

        Yeah, patch looks correct as ZINVALIDSTATE is not handled when converting error num to Exception. Looks like it was simply forgotten because coressponding exception InvalidStateException is declared.

        Show
        Nikita Vetoshkin added a comment - Yeah, patch looks correct as ZINVALIDSTATE is not handled when converting error num to Exception. Looks like it was simply forgotten because coressponding exception InvalidStateException is declared.
        Hide
        Flavio Junqueira added a comment -

        Patch didn't apply cleanly so I have updated and uploaded new patches.

        Show
        Flavio Junqueira added a comment - Patch didn't apply cleanly so I have updated and uploaded new patches.
        Hide
        Flavio Junqueira added a comment -

        +1, trunk Committed revision 1529013. b3.4 Committed revision 1529012.

        Show
        Flavio Junqueira added a comment - +1, trunk Committed revision 1529013. b3.4 Committed revision 1529012.
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1635//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/12606651/ZOOKEEPER-732-trunk.patch against trunk revision 1529013. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1635//console This message is automatically generated.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in ZooKeeper-trunk #2078 (See https://builds.apache.org/job/ZooKeeper-trunk/2078/)
        ZOOKEEPER-732. Improper translation of error into Python exception (Andrei Savu, Lei Zhang, fpj via fpj) (fpj: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1529013)

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/contrib/zkpython/src/c/zookeeper.c
        Show
        Hudson added a comment - SUCCESS: Integrated in ZooKeeper-trunk #2078 (See https://builds.apache.org/job/ZooKeeper-trunk/2078/ ) ZOOKEEPER-732 . Improper translation of error into Python exception (Andrei Savu, Lei Zhang, fpj via fpj) (fpj: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1529013 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/contrib/zkpython/src/c/zookeeper.c
        Hide
        Flavio Junqueira added a comment -

        Closing issues after releasing 3.4.6.

        Show
        Flavio Junqueira added a comment - Closing issues after releasing 3.4.6.

          People

          • Assignee:
            Lei Zhang
            Reporter:
            Gustavo Niemeyer
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development