Avoid importing urllib3 on cli50 startup

cli50/__main__.py

@@ -5,12 +5,14 @@
 
 import argparse
 import gettext
+import json
 import os
 import re
-import requests
 import shutil
 import subprocess
 import tzlocal
+import urllib.error
+import urllib.request
 
 from importlib.resources import files
 from packaging import version
@@ -59,19 +61,21 @@ def main():
     # Check PyPI for newer version
     if __version__ and not args["fast"]:
         try:
-            release = max(requests.get("https://pypi.org/pypi/cli50/json").json()["releases"], key=version.parse)
-            assert release <= __version__
-        except requests.RequestException:
+            release = max(pypi_releases(), key=version.parse)
+            if version.parse(release) <= version.parse(__version__):
+                release = None
+        except (OSError, urllib.error.URLError, json.JSONDecodeError, KeyError, TypeError, ValueError):
             pass
-        except AssertionError:
-            try:
-                response = input("A newer version of cli50 is available. Upgrade now? [Y/n] ")
-            except EOFError:
-                pass
-            else:
-                if response.strip().lower() not in ["n", "no"]:
-                    print("Run `pip3 install --upgrade cli50` to upgrade. Then re-run cli50.")
-                    sys.exit(0)
+        else:
+            if release is not None:
+                try:
+                    response = input("A newer version of cli50 is available. Upgrade now? [Y/n] ")
+                except EOFError:
+                    pass
+                else:
+                    if response.strip().lower() not in ["n", "no"]:
+                        print("Run `pip3 install --upgrade cli50` to upgrade. Then re-run cli50.")
+                        sys.exit(0)
 
     # Check if Docker installed
     if not shutil.which("docker"):
@@ -170,7 +174,6 @@ def main():
     if not args["fast"]:
 
         # Remote manifest
-        import json
         try:
             RemoteManifest = json.loads(subprocess.check_output([
                 "docker", "manifest", "inspect", f"{IMAGE}:{args['tag']}", "--verbose"
@@ -323,7 +326,6 @@ def ports(container):
 
 def pull(image, tag):
     """Pull image as needed."""
-    import json
     try:
 
         # Get the latest manifest from registry
@@ -335,13 +337,20 @@ def pull(image, tag):
         localImageId = json.loads(subprocess.check_output([
             "docker", "inspect", f"{image}:{tag}"], stderr=subprocess.DEVNULL).decode("utf-8"))[0]['Id']
 
-        # Pull latest if local image id does not match any digest in the manifest
-        assert localImageId in [manifest['SchemaV2Manifest']['config']['digest'] for manifest in RemoteManifest] == True
+        if localImageId in [manifest['SchemaV2Manifest']['config']['digest'] for manifest in RemoteManifest]:
+            return
+
+    except (OSError, subprocess.CalledProcessError):
+        pass
+
+    # Pull image
+    subprocess.call(["docker", "pull", f"{image}:{tag}"], stderr=subprocess.DEVNULL)
 
-    except (AssertionError, requests.exceptions.ConnectionError, subprocess.CalledProcessError):
 
-        # Pull image
-        subprocess.call(["docker", "pull", f"{image}:{tag}"], stderr=subprocess.DEVNULL)
+def pypi_releases():
+    """Return release versions published to PyPI."""
+    with urllib.request.urlopen("https://pypi.org/pypi/cli50/json", timeout=10) as response:
+        return json.loads(response.read().decode("utf-8"))["releases"]
 
 
 if __name__ == "__main__":

setup.py

@@ -15,7 +15,7 @@
     description="This is CS50 CLI, with which you can mount a directory inside of an Ubuntu container.",
     long_description=open("README.md").read(),
     license="GPLv3",
-    install_requires=["inflect", "packaging", "requests", "tzlocal"],
+    install_requires=["inflect", "packaging", "tzlocal"],
     keywords="cli50",
     name="cli50",
     python_requires=">=3.8",

tests/test_main.py

@@ -0,0 +1,102 @@
+import io
+import json
+import os
+import argparse
+import subprocess
+import unittest
+from unittest import mock
+
+from cli50 import __main__
+
+
+class PypiReleasesTestCase(unittest.TestCase):
+    def test_pypi_releases_uses_standard_library_response(self):
+        payload = io.BytesIO(b'{"releases": {"8.0.1": [], "8.0.0": []}}')
+
+        with mock.patch("urllib.request.urlopen", return_value=payload) as urlopen:
+            self.assertEqual(__main__.pypi_releases(), {"8.0.1": [], "8.0.0": []})
+
+        urlopen.assert_called_once_with("https://pypi.org/pypi/cli50/json", timeout=10)
+
+    def test_main_ignores_invalid_pypi_json(self):
+        self._assert_update_check_failure_is_ignored(
+            json.JSONDecodeError("Expecting value", "", 0)
+        )
+
+    def test_main_ignores_missing_pypi_releases(self):
+        self._assert_update_check_failure_is_ignored(KeyError("releases"))
+
+    def test_main_compares_versions_semantically(self):
+        args = argparse.Namespace(
+            directory=os.getcwd(),
+            dotfile=[],
+            fast=False,
+            jekyll=False,
+            login=False,
+            port=[],
+            stop=False,
+            tag=__main__.TAG,
+        )
+
+        with mock.patch.object(__main__, "__version__", "10.0.0"), \
+             mock.patch("argparse.ArgumentParser.parse_args", return_value=args), \
+             mock.patch.object(__main__, "pypi_releases", return_value={"9.9.9": []}), \
+             mock.patch("shutil.which", return_value=None), \
+             mock.patch("builtins.input") as prompt, \
+             self.assertRaises(SystemExit) as raised:
+            __main__.main()
+
+        self.assertEqual(raised.exception.code, 2)
+        prompt.assert_not_called()
+
+    def test_pull_falls_back_when_manifest_lookup_fails(self):
+        error = subprocess.CalledProcessError(1, ["docker", "manifest", "inspect"])
+
+        with mock.patch("subprocess.check_output", side_effect=error), \
+             mock.patch("subprocess.call") as docker_pull:
+            __main__.pull("cs50/cli", "latest")
+
+        docker_pull.assert_called_once_with(
+            ["docker", "pull", "cs50/cli:latest"], stderr=__main__.subprocess.DEVNULL
+        )
+
+    def test_pull_falls_back_when_manifest_command_cannot_run(self):
+        with mock.patch("subprocess.check_output", side_effect=OSError("docker unavailable")), \
+             mock.patch("subprocess.call") as docker_pull:
+            __main__.pull("cs50/cli", "latest")
+
+        docker_pull.assert_called_once_with(
+            ["docker", "pull", "cs50/cli:latest"], stderr=__main__.subprocess.DEVNULL
+        )
+
+    def test_pull_does_not_hide_invalid_manifest_data(self):
+        manifest = b'{"unexpected": []}'
+        local = b'[{"Id": "sha256:test"}]'
+
+        with mock.patch("subprocess.check_output", side_effect=[manifest, local]), \
+             mock.patch("subprocess.call") as docker_pull, \
+             self.assertRaises((KeyError, TypeError)):
+            __main__.pull("cs50/cli", "latest")
+
+        docker_pull.assert_not_called()
+
+    def _assert_update_check_failure_is_ignored(self, error):
+        args = argparse.Namespace(
+            directory=os.getcwd(),
+            dotfile=[],
+            fast=False,
+            jekyll=False,
+            login=False,
+            port=[],
+            stop=False,
+            tag=__main__.TAG,
+        )
+
+        with mock.patch.object(__main__, "__version__", "8.0.1"), \
+             mock.patch("argparse.ArgumentParser.parse_args", return_value=args), \
+             mock.patch.object(__main__, "pypi_releases", side_effect=error), \
+             mock.patch("shutil.which", return_value=None), \
+             self.assertRaises(SystemExit) as raised:
+            __main__.main()
+
+        self.assertEqual(raised.exception.code, 2)