Repository: civisanalytics/civis-python

Issue:
Test with newer version of click and update dependencies
civis-python requires `click` version 6, but there's now a v7. We should test with it and update the `requirements.txt` file.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4949c98..4f7926c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,13 +4,22 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+### Fixed
+- Fix unintentional dependency on scikit-learn for `parallel` module tests. (#245, #303)
+
 ### Changed
 - Loosened version requirements of `pyyaml` to include `pyyaml<=5.99`. (#293)
 - Loosened version requirement of `jsonref` to include `0.2` to fix a
   DeprecationWarning under Python 3.7. (#295)
 - Changed pubnub version requirement in requirements.txt to match setup.py
   (#295)
+- Loosened version requirements of `click` to include v7 and `jsonschema`
+  to include v3. (#286, #300)
 - Surfaced `civis.io.split_schema_tablename` in the Sphinx docs. (#294)
+- Loosen `joblib` version requirement to include v0.13 and add code to
+  the Civis joblib backend which newer versions of `joblib` can take
+  advantage of. Also loosed version requirement on `cloudpickle` to
+  include v1. (#296, #299)
 
 ## 1.10.0 - 2019-04-09
 ### Added
diff --git a/civis/parallel.py b/civis/parallel.py
index 47fbafd..bb67edb 100644
--- a/civis/parallel.py
+++ b/civis/parallel.py
@@ -9,6 +9,7 @@ import logging
 import os
 import pickle
 import time
+import warnings
 
 import cloudpickle
 from joblib._parallel_backends import ParallelBackendBase
@@ -23,11 +24,20 @@ from civis.compat import TemporaryDirectory
 from civis.futures import _ContainerShellExecutor, CustomScriptExecutor
 
 try:
-    from sklearn.externals.joblib import (
-        register_parallel_backend as _sklearn_reg_para_backend)
-    NO_SKLEARN = False
+    with warnings.catch_warnings():
+        # Ignore the warning: "DeprecationWarning: sklearn.externals.joblib is
+        # deprecated in 0.21 and will be removed in 0.23. Please import this
+        # functionality directly from joblib, which can be installed with:
+        # pip install joblib. If this warning is raised when loading pickled
+        # models, you may need to re-serialize those models with
+        # scikit-learn 0.21+."
+        warnings.simplefilter('ignore', DeprecationWarning)
+        from sklearn.externals.joblib import (
+            register_parallel_backend as _sklearn_reg_para_backend)
+        NO_SKLEARN = False
 except ImportError:
     NO_SKLEARN = True
+    _sklearn_reg_para_backend = None
 
 log = logging.getLogger(__name__)
 _THIS_DIR = os.path.dirname(os.path.realpath(__file__))
@@ -716,9 +726,14 @@ class _CivisBackendResult:
 
         return _fetch_result
 
-    def get(self):
+    def get(self, timeout=None):
         """Block and return the result of the job
 
+        Parameters
+        ----------
+        timeout: float, optional
+            If provided, wait this many seconds before issuing a TimeoutError
+
         Returns
         -------
         The output of the function which ``joblib`` ran via Civis
@@ -736,7 +751,7 @@ class _CivisBackendResult:
         """
         if self.result is None:
             # Wait for the script to complete.
-            wait([self._future])
+            wait([self._future], timeout=timeout)
             self.result = self._future.remote_func_output
 
         if self._future.exception() or not self._future.result_fetched:
@@ -761,11 +776,16 @@ class _CivisBackend(ParallelBackendBase):
 
     Users should interact with this through ``make_backend_factory``.
     """
+    uses_threads = False
+    supports_sharedmem = False
+    supports_timeout = True
+
     def __init__(self, setup_cmd=_DEFAULT_SETUP_CMD,
                  from_template_id=None,
                  max_submit_retries=0,
                  client=None,
                  remote_backend='sequential',
+                 nesting_level=0,
                  **executor_kwargs):
         self.setup_cmd = setup_cmd
         self.from_template_id = from_template_id
@@ -773,6 +793,7 @@ class _CivisBackend(ParallelBackendBase):
         self.client = client
         self.remote_backend = remote_backend
         self.executor_kwargs = executor_kwargs
+        self.nesting_level = nesting_level
         self._init_civis_backend()
 
     @classmethod
@@ -821,6 +842,10 @@ class _CivisBackend(ParallelBackendBase):
         if not ensure_ready:
             self.executor.shutdown(wait=False)
 
+    def terminate(self):
+        """Shutdown the workers and free the shared memory."""
+        return self.abort_everything(ensure_ready=True)
+
     def apply_async(self, func, callback=None):
         """Schedule func to be run
         """
diff --git a/requirements.txt b/requirements.txt
index 6bffa78..27a97ed 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,13 +1,13 @@
 pyyaml>=3.0,<=5.99
-click>=6.0,<=6.99
+click>=6.0,<=7
 funcsigs==1.0.2 ; python_version == '2.7'
 future>=0.16,<=0.99 ; python_version == '2.7'
 futures==3.1.1 ; python_version == '2.7'
 jsonref>=0.1,<=0.2
 requests>=2.12.0,==2.*
-jsonschema>=2.5.1,==2.*
+jsonschema>=2.5.1,<=3
 functools32>=3.2,<=3.99 ; python_version == '2.7'
 six>=1.10,<=1.99
-joblib~=0.11
+joblib>=0.11,<=0.13
 pubnub>=4.0,<=4.99
-cloudpickle~=0.2
+cloudpickle<=0.2,<=1
diff --git a/setup.py b/setup.py
index 85bb9e0..1d52c97 100644
--- a/setup.py
+++ b/setup.py
@@ -52,14 +52,14 @@ def main():
         long_description=README,
         install_requires=[
             'pyyaml>=3.0,<=5.99',
-            'click>=6.0,<=6.99',
+            'click>=6.0,<=7',
             'jsonref>=0.1.0,<=0.2',
             'requests>=2.12.0,==2.*',
-            'jsonschema>=2.5.1,==2.*',
+            'jsonschema>=2.5.1,<=3',
             'six>=1.10,<=1.99',
-            'joblib>=0.11,<=0.11.99',
+            'joblib>=0.11,<=0.13.99',
             'pubnub>=4.0,<=4.99',
-            'cloudpickle>=0.2.0,<=0.99999',
+            'cloudpickle>=0.2.0,<=1.99999',
         ],
         extras_require={
             ':python_version=="2.7"': [


Test patch:
diff --git a/civis/tests/test_parallel.py b/civis/tests/test_parallel.py
index 846fb52..f9d75fe 100644
--- a/civis/tests/test_parallel.py
+++ b/civis/tests/test_parallel.py
@@ -13,6 +13,7 @@ import requests
 
 import civis.parallel
 from civis.futures import ContainerFuture
+from civis.tests import create_client_mock
 
 
 @pytest.fixture
@@ -67,7 +68,7 @@ def _test_retries_helper(num_failures, max_submit_retries,
                          mock_custom_exec_cls, mock_executor_cls):
 
     mock_file_to_civis.return_value = 0
-    mock_result_cls.get.return_value = 123
+    mock_result_cls.return_value.get.return_value = [123]
 
     # A function to raise fake API errors the first num_failures times it is
     # called.
@@ -87,13 +88,17 @@ def _test_retries_helper(num_failures, max_submit_retries,
         factory = civis.parallel.make_backend_template_factory(
             from_template_id=from_template_id,
             max_submit_retries=max_submit_retries,
-            client=mock.Mock())
+            client=create_client_mock())
     else:
         factory = civis.parallel.make_backend_factory(
-            max_submit_retries=max_submit_retries, client=mock.Mock())
+            max_submit_retries=max_submit_retries, client=create_client_mock())
     register_parallel_backend('civis', factory)
     with parallel_backend('civis'):
-        parallel = Parallel(n_jobs=5, pre_dispatch='n_jobs')
+        # NB: joblib >v0.11 relies on callbacks from the result object to
+        # decide when it's done consuming inputs. We've mocked the result
+        # object here, so Parallel must be called either with n_jobs=1 or
+        # pre_dispatch='all' to consume the inputs all at once.
+        parallel = Parallel(n_jobs=1, pre_dispatch='n_jobs')
         if should_fail:
             with pytest.raises(civis.parallel.JobSubmissionError):
                 parallel(delayed(sqrt)(i ** 2) for i in range(3))
@@ -116,7 +121,11 @@ def test_template_submit(mock_file, mock_result, mock_pool):
     n_calls = 3
     register_parallel_backend('civis', factory)
     with parallel_backend('civis'):
-        parallel = Parallel(n_jobs=5, pre_dispatch='n_jobs')
+        # NB: joblib >v0.11 relies on callbacks from the result object to
+        # decide when it's done consuming inputs. We've mocked the result
+        # object here, so Parallel must be called either with n_jobs=1 or
+        # pre_dispatch='all' to consume the inputs all at once.
+        parallel = Parallel(n_jobs=1, pre_dispatch='n_jobs')
         parallel(delayed(sqrt)(i ** 2) for i in range(n_calls))
 
     assert mock_file.call_count == 3, "Upload 3 functions to run"
@@ -267,7 +276,7 @@ def test_infer_update_args(mock_make_factory, mock_job):
 
 
 @mock.patch.object(civis.parallel, 'make_backend_factory')
-def test_infer_from_custom_job(mock_make_factory):
+def test_infer_from_custom_job(mock_make_factory, mock_job):
     # Test that `infer_backend_factory` can find needed
     # parameters if it's run inside a custom job created
     # from a template.
@@ -279,14 +288,13 @@ def test_infer_from_custom_job(mock_make_factory):
                                 docker_image_name='image_name',
                                 docker_image_tag='tag',
                                 repo_http_uri='cabbage', repo_ref='servant'))
-    mock_script = mock_job()
     mock_template = Response(dict(id=999, script_id=171))
 
     def _get_container(job_id):
         if int(job_id) == 42:
             return mock_custom
         elif int(job_id) == 171:
-            return mock_script
+            return mock_job
         else:
             raise ValueError("Got job_id {}".format(job_id))
 
@@ -315,7 +323,7 @@ def test_infer_from_custom_job(mock_make_factory):
                        'hidden': True,
                        'remote_backend': 'sequential'}
     for key in civis.parallel.KEYS_TO_INFER:
-        expected_kwargs[key] = mock_script[key]
+        expected_kwargs[key] = mock_job[key]
     mock_make_factory.assert_called_once_with(**expected_kwargs)