Uploaded image for project: 'Libcloud'
  1. Libcloud
  2. LIBCLOUD-321

libcloud.test.compute.test_deployment.DeploymentTests.test_script_file_deployment() fails with Python 3.3

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.13.0
    • Component/s: None
    • Labels:
      None

      Description

      Libcloud-0.12.4 introduced libcloud.test.compute.test_deployment.DeploymentTests.test_script_file_deployment(), which fails with Python 3.3.

      ======================================================================
      ERROR: test_script_file_deployment (libcloud.test.compute.test_deployment.DeploymentTests)
      ----------------------------------------------------------------------
      Traceback (most recent call last):
        File "/tmp/apache-libcloud-0.12.4/libcloud/test/compute/test_deployment.py", line 126, in test_script_file_deployment
          sfd1 = ScriptFileDeployment(script_file=file_path)
        File "/tmp/apache-libcloud-0.12.4/libcloud/compute/deployment.py", line 193, in __init__
          delete=delete)
        File "/tmp/apache-libcloud-0.12.4/libcloud/compute/deployment.py", line 133, in __init__
          argument_value=script)
        File "/tmp/apache-libcloud-0.12.4/libcloud/compute/deployment.py", line 52, in _get_string_value
          'object' % (argument_name))
      TypeError: script argument must be a string or a file-like object
      
      ----------------------------------------------------------------------
      

      libcloud.utils.py3.basestring is defined as str for Python 3.
      Deployment._get_string_value() accepts argument_value, which is libcloud.utils.py3.basestring (i.e. str), but ScriptFileDeployment.__init__() opens a file in binary mode, so the result of reading is bytes, not str.

      Potential fix:

      --- libcloud/compute/deployment.py
      +++ libcloud/compute/deployment.py
      @@ -185,7 +185,7 @@
               @type delete: C{bool}
               @keyword delete: Whether to delete the script on completion.
               """
      -        with open(script_file, 'rb') as fp:
      +        with open(script_file, 'r') as fp:
                   content = fp.read()
       
               super(ScriptFileDeployment, self).__init__(script=content,
      --- libcloud/test/compute/test_deployment.py
      +++ libcloud/test/compute/test_deployment.py
      @@ -115,12 +115,8 @@
                               client=MockClient(hostname='localhost')))
       
           def test_script_file_deployment(self):
      -        # TODO: Fix 3.2 compatibility
      -        if PY32:
      -            return
      -
               file_path = os.path.abspath(__file__)
      -        with open(file_path, 'rb') as fp:
      +        with open(file_path, 'r') as fp:
                   content = fp.read()
       
               sfd1 = ScriptFileDeployment(script_file=file_path)
      

      Alternative fix would be to open file in binary mode and (only when running with Python 3) to decode content from bytes to str.

      1. libcloud.patch
        1 kB
        Arfrever Frehtes Taifersar Arahesis

        Activity

        Hide
        kami Tomaz Muraus added a comment -

        Done.

        Show
        kami Tomaz Muraus added a comment - Done.
        Hide
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment -

        Please backport this change to 0.12.x branch.

        Show
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment - Please backport this change to 0.12.x branch.
        Hide
        kami Tomaz Muraus added a comment -

        Thanks.

        This patch works correctly and I've merged it into trunk.

        The whole Python 3 compatibility layer is a bit of a mess. I plan to do some cleanups and improvements in the near future and hopefully we can get rid more of nasty if's like this then.

        Show
        kami Tomaz Muraus added a comment - Thanks. This patch works correctly and I've merged it into trunk. The whole Python 3 compatibility layer is a bit of a mess. I plan to do some cleanups and improvements in the near future and hopefully we can get rid more of nasty if's like this then.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 1471675 from Tomaz Muraus
        [ https://svn.apache.org/r1471675 ]

        Fix Python 3 compatibility issue in the ScriptFileDeployment class.

        Part of LIBCLOUD-321, contributed by Arfrever Frehtes Taifersar Arahesis.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 1471675 from Tomaz Muraus [ https://svn.apache.org/r1471675 ] Fix Python 3 compatibility issue in the ScriptFileDeployment class. Part of LIBCLOUD-321 , contributed by Arfrever Frehtes Taifersar Arahesis.
        Hide
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment -

        Please try this patch.

        Show
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment - Please try this patch.
        Hide
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment -

        Yes.
        I now noticed that this test fails with C locale and Python 3. It passes with a UTF-8 locale.

        Show
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment - Yes. I now noticed that this test fails with C locale and Python 3. It passes with a UTF-8 locale.
        Hide
        kami Tomaz Muraus added a comment -

        Yeah, I will change it to use the code tag

        As far as the test goes - did you remove

        # TODO: Fix 3.2 compatibility
        if PY32:
            return
        

        from the test_script_file_deployment test?

        Show
        kami Tomaz Muraus added a comment - Yeah, I will change it to use the code tag As far as the test goes - did you remove # TODO: Fix 3.2 compatibility if PY32: return from the test_script_file_deployment test?
        Hide
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment -

        Better use {code} than {quote} .
        This test really passed for me with Python 3.2 and this patch applied.

        Show
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment - Better use {code} than {quote} . This test really passed for me with Python 3.2 and this patch applied.
        Hide
        kami Tomaz Muraus added a comment - - edited

        Doesn't work here because there is an issue with how we do string type detection in libcloud.compute.deployment and what we set basestring to in libcloud.utils.py3.

        if not isinstance(argument_value, basestring) and \
        not hasattr(argument_value, 'read'):

        svn diff
        Index: libcloud/compute/deployment.py
        ===================================================================
        --- libcloud/compute/deployment.py	(revision 1471588)
        +++ libcloud/compute/deployment.py	(working copy)
        @@ -104,7 +104,7 @@
                 """
                 perms = int(oct(os.stat(self.source).st_mode)[4:], 8)
         
        -        with open(self.source, 'rb') as fp:
        +        with open(self.source, 'r') as fp:
                     content = fp.read()
         
                 client.put(path=self.target, chmod=perms,
        Index: libcloud/test/compute/test_deployment.py
        ===================================================================
        --- libcloud/test/compute/test_deployment.py	(revision 1471588)
        +++ libcloud/test/compute/test_deployment.py	(working copy)
        @@ -115,12 +115,8 @@
                                 client=MockClient(hostname='localhost')))
         
             def test_script_file_deployment(self):
        -        # TODO: Fix 3.2 compatibility
        -        if PY32:
        -            return
        -
                 file_path = os.path.abspath(__file__)
        -        with open(file_path, 'rb') as fp:
        +        with open(file_path, 'r') as fp:
                     content = fp.read()
         
                 sfd1 = ScriptFileDeployment(script_file=file_path)
        
        tox -e py3
        
        ======================================================================
        ERROR: test_script_file_deployment (libcloud.test.compute.test_deployment.DeploymentTests)
        ----------------------------------------------------------------------
        Traceback (most recent call last):
          File "/w/lc/t1/libcloud/test/compute/test_deployment.py", line 122, in test_script_file_deployment
            sfd1 = ScriptFileDeployment(script_file=file_path)
          File "/w/lc/t1/libcloud/compute/deployment.py", line 193, in __init__
            delete=delete)
          File "/w/lc/t1/libcloud/compute/deployment.py", line 133, in __init__
            argument_value=script)
          File "/w/lc/t1/libcloud/compute/deployment.py", line 52, in _get_string_value
            'object' % (argument_name))
        TypeError: script argument must be a string or a file-like object
        
        ----------------------------------------------------------------------
        Ran 2696 tests in 8.717s
        
        FAILED (errors=1)
        [TOX] ERROR: InvocationError: '.tox/py32/bin/python setup.py test'
        
        Show
        kami Tomaz Muraus added a comment - - edited Doesn't work here because there is an issue with how we do string type detection in libcloud.compute.deployment and what we set basestring to in libcloud.utils.py3. if not isinstance(argument_value, basestring) and \ not hasattr(argument_value, 'read'): svn diff Index: libcloud/compute/deployment.py =================================================================== --- libcloud/compute/deployment.py (revision 1471588) +++ libcloud/compute/deployment.py (working copy) @@ -104,7 +104,7 @@ """ perms = int (oct(os.stat(self.source).st_mode)[4:], 8) - with open(self.source, 'rb') as fp: + with open(self.source, 'r') as fp: content = fp.read() client.put(path=self.target, chmod=perms, Index: libcloud/test/compute/test_deployment.py =================================================================== --- libcloud/test/compute/test_deployment.py (revision 1471588) +++ libcloud/test/compute/test_deployment.py (working copy) @@ -115,12 +115,8 @@ client=MockClient(hostname='localhost'))) def test_script_file_deployment(self): - # TODO: Fix 3.2 compatibility - if PY32: - return - file_path = os.path.abspath(__file__) - with open(file_path, 'rb') as fp: + with open(file_path, 'r') as fp: content = fp.read() sfd1 = ScriptFileDeployment(script_file=file_path) tox -e py3 ====================================================================== ERROR: test_script_file_deployment (libcloud.test.compute.test_deployment.DeploymentTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/w/lc/t1/libcloud/test/compute/test_deployment.py" , line 122, in test_script_file_deployment sfd1 = ScriptFileDeployment(script_file=file_path) File "/w/lc/t1/libcloud/compute/deployment.py" , line 193, in __init__ delete=delete) File "/w/lc/t1/libcloud/compute/deployment.py" , line 133, in __init__ argument_value=script) File "/w/lc/t1/libcloud/compute/deployment.py" , line 52, in _get_string_value 'object' % (argument_name)) TypeError: script argument must be a string or a file-like object ---------------------------------------------------------------------- Ran 2696 tests in 8.717s FAILED (errors=1) [TOX] ERROR: InvocationError: '.tox/py32/bin/python setup.py test'
        Hide
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment -

        This patch works also with Python 3.2.

        Show
        arfrever Arfrever Frehtes Taifersar Arahesis added a comment - This patch works also with Python 3.2.
        Hide
        kami Tomaz Muraus added a comment -

        Good catch, thanks.

        This fixes an issue for Python 3.3, but not for 3.2. I will look into this and try to come up with a solution which works with all the supported versions asap.

        Show
        kami Tomaz Muraus added a comment - Good catch, thanks. This fixes an issue for Python 3.3, but not for 3.2. I will look into this and try to come up with a solution which works with all the supported versions asap.

          People

          • Assignee:
            kami Tomaz Muraus
            Reporter:
            arfrever Arfrever Frehtes Taifersar Arahesis
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development