SG-38306 Python2 Removal - Part 7 - various (#404)

Author: julien-lang

README.md

@@ -37,7 +37,7 @@ Integration and unit tests are provided.
     - (Note: Running `pip install -r tests/ci_requirements.txt` will install this package)
 - A `tests/config` file (you can copy an example from `tests/example_config`).
 - Tests can be run individually like this: `nosetests --config="nose.cfg" tests/test_client.py`
-    - Make sure to not forget the `--config="nose.cfg"` option. This option tells nose to use our config file.  This will exclude python 2- and 3-specific files in the `/lib` directory, preventing a failure from being reported by nose for compilation due to incompatible syntax in those files.
+    - Make sure to not forget the `--config="nose.cfg"` option. This option tells nose to use our config file.
 - `test_client` and `tests_unit` use mock server interaction and do not require a Flow Production Tracking instance to be available (no modifications to `tests/config` are necessary).
 - `test_api` and `test_api_long` *do* require a Flow Production Tracking instance, with a script key available for the tests. The server and script user values must be supplied in the `tests/config` file. The tests will add test data to your server based on information in your config. This data will be manipulated by the tests, and should not be used for other purposes.
 - To run all of the tests, use the shell script `run-tests`.

azure-pipelines-templates/run-tests.yml

@@ -33,9 +33,9 @@ parameters:
 
 jobs:
   # The job will be named after the OS and Azure will suffix the strategy to make it unique
-  # so we'll have a job name "Windows Python 2.7" for example. What's a strategy? Strategies are the
-  # name of the keys under the strategy.matrix scope. So for each OS we'll have "<OS> Python 2.7" and
-  # "<OS> Python 3.7".
+  # so we'll have a job name "Windows Python 3.9" for example. What's a strategy? Strategies are the
+  # name of the keys under the strategy.matrix scope. So for each OS we'll have "<OS> Python 3.9" and
+  # "<OS> Python 3.10".
 - job: ${{ parameters.name }}
   pool:
     vmImage: ${{ parameters.vm_image }}
@@ -68,8 +68,8 @@ jobs:
       versionSpec: '$(python.version)'
       addToPath: True
 
-  # Install all dependencies needed for running the tests. This command is good for
-  # Python 2 and 3, but also for all OSes
+  # Install all dependencies needed for running the tests. This command is good
+  # for all OSes
   - script: |
       python -m pip install --upgrade pip setuptools wheel
       python -m pip install -r tests/ci_requirements.txt

shotgun_api3/shotgun.py

@@ -35,7 +35,7 @@
 import json
 import http.client  # Used for secure file upload
 import http.cookiejar  # used for attachment upload
-import io  # used for attachment upload
+import io
 import logging
 import mimetypes
 import os
@@ -49,14 +49,14 @@
 import urllib.parse
 import urllib.request
 import uuid  # used for attachment upload
+import xml.etree.ElementTree
 
 # Import Error and ResponseError (even though they're unused in this file) since they need
 # to be exposed as part of the API.
 from xmlrpc.client import Error, ProtocolError, ResponseError  # noqa
 
 # Python 2/3 compatibility
 from .lib import six
-from .lib import sgsix
 from .lib import sgutils
 from .lib.httplib2 import Http, ProxyInfo, socks
 from .lib.sgtimezone import SgTimezone
@@ -329,7 +329,7 @@ class ClientCapabilities(object):
         ``windows``, or ``None`` (if the current platform couldn't be determined).
     :ivar str local_path_field: The PTR field used for local file paths. This is calculated using
         the value of ``platform``. Ex. ``local_path_mac``.
-    :ivar str py_version: Simple version of Python executable as a string. Eg. ``2.7``.
+    :ivar str py_version: Simple version of Python executable as a string. Eg. ``3.9``.
     :ivar str ssl_version: Version of OpenSSL installed. Eg. ``OpenSSL 1.0.2g  1 Mar 2016``. This
         info is only available in Python 2.7+ if the ssl module was imported successfully.
         Defaults to ``unknown``
@@ -567,18 +567,6 @@ def __init__(
                 :class:`~shotgun_api3.MissingTwoFactorAuthenticationFault` will be raised if the
                 ``auth_token`` is invalid.
             .. todo: Add this info to the Authentication section of the docs
-
-        .. note:: A note about proxy connections: If you are using Python <= v2.6.2, HTTPS
-            connections through a proxy server will not work due to a bug in the :mod:`urllib2`
-            library (see http://bugs.python.org/issue1424152). This will affect upload and
-            download-related methods in the Shotgun API (eg. :meth:`~shotgun_api3.Shotgun.upload`,
-            :meth:`~shotgun_api3.Shotgun.upload_thumbnail`,
-            :meth:`~shotgun_api3.Shotgun.upload_filmstrip_thumbnail`,
-            :meth:`~shotgun_api3.Shotgun.download_attachment`. Normal CRUD methods for passing JSON
-            data should still work fine. If you cannot upgrade your Python installation, you can see
-            the patch merged into Python v2.6.3 (http://hg.python.org/cpython/rev/0f57b30a152f/) and
-            try and hack it into your installation but YMMV. For older versions of Python there
-            are other patches that were proposed in the bug report that may help you as well.
         """
 
         # verify authentication arguments
@@ -617,13 +605,7 @@ def __init__(
             if script_name is not None or api_key is not None:
                 raise ValueError("cannot provide an auth_code with script_name/api_key")
 
-        # Can't use 'all' with python 2.4
-        if (
-            len(
-                [x for x in [session_token, script_name, api_key, login, password] if x]
-            )
-            == 0
-        ):
+        if not any([session_token, script_name, api_key, login, password]):
             if connect:
                 raise ValueError(
                     "must provide login/password, session_token or script_name/api_key"
@@ -2879,8 +2861,7 @@ def download_attachment(self, attachment=False, file_path=None, attachment_id=No
                 This parameter exists only for backwards compatibility for scripts specifying
                 the parameter with keywords.
         :returns: If ``file_path`` is provided, returns the path to the file on disk.  If
-            ``file_path`` is ``None``, returns the actual data of the file, as str in Python 2 or
-            bytes in Python 3.
+            ``file_path`` is ``None``, returns the actual data of the file, as bytes.
         :rtype: str | bytes
         """
         # backwards compatibility when passed via keyword argument
@@ -2941,12 +2922,13 @@ def download_attachment(self, attachment=False, file_path=None, attachment_id=No
                         ]
 
                         if body:
-                            xml = "".join(body)
-                            # Once python 2.4 support is not needed we can think about using
-                            # elementtree. The doc is pretty small so this shouldn't be an issue.
-                            match = re.search("<Message>(.*)</Message>", xml)
-                            if match:
-                                err += " - %s" % (match.group(1))
+                            try:
+                                root = xml.etree.ElementTree.fromstring("".join(body))
+                                message_elem = root.find(".//Message")
+                                if message_elem is not None and message_elem.text:
+                                    err = f"{err} - {message_elem.text}"
+                            except xml.etree.ElementTree.ParseError:
+                                err = f"{err}\n{''.join(body)}\n"
                 elif e.code == 409 or e.code == 410:
                     # we may be dealing with a file that is pending/failed a malware scan, e.g:
                     # 409: This file is undergoing a malware scan, please try again in a few minutes
@@ -4693,17 +4675,12 @@ class FormPostHandler(urllib.request.BaseHandler):
     handler_order = urllib.request.HTTPHandler.handler_order - 10  # needs to run first
 
     def http_request(self, request):
-        # get_data was removed in 3.4. since we're testing against 3.6 and
-        # 3.7, this should be sufficient.
-        if six.PY3:
-            data = request.data
-        else:
-            data = request.get_data()
+        data = request.data
         if data is not None and not isinstance(data, str):
             files = []
             params = []
             for key, value in data.items():
-                if isinstance(value, sgsix.file_types):
+                if isinstance(value, io.IOBase):
                     files.append((key, value))
                 else:
                     params.append((key, value))
@@ -4714,12 +4691,8 @@ def http_request(self, request):
                 boundary, data = self.encode(params, files)
                 content_type = "multipart/form-data; boundary=%s" % boundary
                 request.add_unredirected_header("Content-Type", content_type)
-            # add_data was removed in 3.4. since we're testing against 3.6 and
-            # 3.7, this should be sufficient.
-            if six.PY3:
-                request.data = data
-            else:
-                request.add_data(data)
+            request.data = data
+
         return request
 
     def encode(self, params, files, boundary=None, buffer=None):

tests/base.py

@@ -176,13 +176,10 @@ def _mock_http(self, data, headers=None, status=None):
             return
 
         if not isinstance(data, str):
-            if six.PY2:
-                data = json.dumps(data, ensure_ascii=False, encoding="utf-8")
-            else:
-                data = json.dumps(
-                    data,
-                    ensure_ascii=False,
-                )
+            data = json.dumps(
+                data,
+                ensure_ascii=False,
+            )
 
         resp_headers = {
             "cache-control": "no-cache",

tests/test_api.py

@@ -32,11 +32,6 @@
 from shotgun_api3.lib import six
 from shotgun_api3.lib.httplib2 import Http
 
-# To mock the correct exception when testion on Python 2 and 3, use the
-# ShotgunSSLError variable from sgsix that contains the appropriate exception
-# class for the current Python version.
-from shotgun_api3.lib.sgsix import ShotgunSSLError
-
 import shotgun_api3
 
 from . import base
@@ -272,44 +267,6 @@ def test_upload_download(self):
             "sg_uploaded_movie",
             tag_list="monkeys, everywhere, send, help",
         )
-        if six.PY2:
-            # In Python2, make sure that non-utf-8 encoded paths raise when they
-            # can't be converted to utf-8.  For Python3, we'll skip these tests
-            # since string encoding is handled differently.
-
-            # We need to touch the file we're going to test with first. We can't
-            # bundle a file with this filename in the repo due to some pip install
-            # problems on Windows. Note that the path below is utf-8 encoding of
-            # what we'll eventually encode as shift-jis.
-            file_path_s = os.path.join(this_dir, "./\xe3\x81\x94.shift-jis")
-            file_path_u = file_path_s.decode("utf-8")
-
-            with open(
-                file_path_u if sys.platform.startswith("win") else file_path_s, "w"
-            ) as fh:
-                fh.write("This is just a test file with some random data in it.")
-
-            self.assertRaises(
-                shotgun_api3.ShotgunError,
-                self.sg.upload,
-                "Version",
-                self.version["id"],
-                file_path_u.encode("shift-jis"),
-                "sg_uploaded_movie",
-                tag_list="monkeys, everywhere, send, help",
-            )
-
-            # But it should work in all cases if a unicode string is used.
-            self.sg.upload(
-                "Version",
-                self.version["id"],
-                file_path_u,
-                "sg_uploaded_movie",
-                tag_list="monkeys, everywhere, send, help",
-            )
-
-            # cleanup
-            os.remove(file_path_u)
 
         # cleanup
         os.remove(file_path)
@@ -2265,7 +2222,7 @@ def my_side_effect2(*args, **kwargs):
     @unittest.mock.patch("shotgun_api3.shotgun.Http.request")
     def test_sha2_error(self, mock_request):
         # Simulate the exception raised with SHA-2 errors
-        mock_request.side_effect = ShotgunSSLError(
+        mock_request.side_effect = ssl.SSLError(
             "[Errno 1] _ssl.c:480: error:0D0C50A1:asn1 "
             "encoding routines:ASN1_item_verify: unknown message digest "
             "algorithm"
@@ -2292,7 +2249,7 @@ def test_sha2_error(self, mock_request):
 
         try:
             self.sg.info()
-        except ShotgunSSLError:
+        except ssl.SSLError:
             # ensure the api has reset the values in the correct fallback behavior
             self.assertTrue(self.sg.config.no_ssl_validation)
             self.assertTrue(shotgun_api3.shotgun.NO_SSL_VALIDATION)
@@ -2305,7 +2262,7 @@ def test_sha2_error(self, mock_request):
     @unittest.mock.patch("shotgun_api3.shotgun.Http.request")
     def test_sha2_error_with_strict(self, mock_request):
         # Simulate the exception raised with SHA-2 errors
-        mock_request.side_effect = ShotgunSSLError(
+        mock_request.side_effect = ssl.SSLError(
             "[Errno 1] _ssl.c:480: error:0D0C50A1:asn1 "
             "encoding routines:ASN1_item_verify: unknown message digest "
             "algorithm"
@@ -2322,7 +2279,7 @@ def test_sha2_error_with_strict(self, mock_request):
 
         try:
             self.sg.info()
-        except ShotgunSSLError:
+        except ssl.SSLError:
             # ensure the api has NOT reset the values in the fallback behavior because we have
             # set the env variable to force validation
             self.assertFalse(self.sg.config.no_ssl_validation)

tests/test_client.py

@@ -651,10 +651,7 @@ def _assert_decode_resonse(self, ensure_ascii, data):
             connect=False,
         )
 
-        if six.PY3:
-            j = json.dumps(d, ensure_ascii=ensure_ascii)
-        else:
-            j = json.dumps(d, ensure_ascii=ensure_ascii, encoding="utf-8")
+        j = json.dumps(d, ensure_ascii=ensure_ascii)
         self.assertEqual(d, sg._decode_response(headers, j))
 
         headers["content-type"] = "text/javascript"