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 -

        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.
        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 - - 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 -

        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 -

        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 -

        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
        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
        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
        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
        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 -

        Done.

        Show
        kami Tomaz Muraus added a comment - Done.

          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