Add Python 3 support #416

jwirjo · 2018-02-21T10:26:32Z

Fixes #138. Added Python3 ClipperPy3TestsDockerfile, ran most integration_tests (run_unittests.sh) in python3 with same output as under python2, but 2 recurring errors.

AmplabJenkins · 2018-02-21T10:41:36Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1032/
Test FAILed.

dcrankshaw · 2018-02-23T17:56:43Z

@jwirjo Can you delete all the extra files you added in bin/? That will make this much easier to review.

dcrankshaw · 2018-02-23T17:57:39Z

Sorry meant to say in clipper_admin/build

dcrankshaw

This looks pretty good. I'm going to play around with this on my machine as well, but I added a first round of comments.

dcrankshaw · 2018-02-23T17:58:41Z

clipper_admin/clipper_admin/deployers/deployer_utils.py

@@ -42,6 +42,7 @@ def save_python_function(name, func):
    c.dump(func)
    serialized_prediction_function = s.getvalue()

+


Revert this change

dcrankshaw · 2018-02-23T17:59:14Z

clipper_admin/clipper_admin/deployers/python.py

        num_replicas=1):
+


Delete extra line

dcrankshaw · 2018-02-23T17:59:21Z

clipper_admin/clipper_admin/deployers/python.py

@@ -72,6 +74,7 @@ def create_endpoint(
        :py:meth:`clipper.ClipperConnection.set_num_replicas`.
    """

+


Delete extra line

dcrankshaw · 2018-02-23T18:00:49Z

clipper_admin/clipper_admin/docker/docker_container_manager.py

+                    lf.write(c.logs(stdout=True, stderr=True))
+            except TypeError:
+                with open(log_file, "wb") as lf:
+                    lf.write(c.logs(stdout=True, stderr=True))


Is this a Python 2 vs 3 thing? If so, instead of trying one way and catching an exception, let's explicitly check for sys.version like you do in the python deployer.

dcrankshaw · 2018-02-23T19:43:19Z

containers/python/rpc.py

+        try:
+            socket.send("", zmq.SNDMORE)
+        except:
+            socket.send("".encode('utf-8'), zmq.SNDMORE)


Is this a Python 2 vs 3 thing? If so, instead of trying one way and catching an exception, let's explicitly check for sys.version like you do in the python deployer and then just use the right version. Throwing and catching an exception can be expensive and the code as-is has unclear semantics.

dcrankshaw · 2018-02-23T19:44:26Z

containers/python/sum_container.py

@@ -22,6 +22,7 @@ def predict_bytes(self, inputs):
        return [str(sum(item)) for item in inputs]

    def predict_strings(self, inputs):
+        print(inputs)


dcrankshaw · 2018-02-23T19:45:27Z

dockerfiles/ClipperPy3TestsDockerfile

+    && git apply ../patches/make_spdlog_compile_linux.patch
+
+
+ENTRYPOINT ["/clipper/bin/ci_checks.sh"]


We'll want to modify this entrypoint command to just run the Python 3-compatible tests, not re-run everything

dcrankshaw · 2018-02-23T19:46:14Z

dockerfiles/Py3ClosureContainerDockerfile.txt

@@ -0,0 +1,13 @@
+ARG CODE_VERSION


The Dockerfile file name should not end in .txt

dcrankshaw · 2018-02-23T19:46:34Z

dockerfiles/Py3RPCDockerfile.txt

@@ -0,0 +1,20 @@
+# This ARG isn't used but prevents warnings in the build script


The Dockerfile file name should not end in .txt

dcrankshaw · 2018-02-23T19:46:56Z

integration-tests/clipper_admin_tests.py

+    #'test_set_num_replicas_for_deployed_model_succeeds',
+    #'test_remove_inactive_containers_succeeds',
+    #'test_stop_models',
+    #'test_python_closure_deploys_successfully',
    'test_register_py_endpoint',


Why are these all commented out?

dcrankshaw · 2018-02-24T00:27:15Z

bin/build_docker_images.sh

@@ -245,16 +245,20 @@ build_images () {
    create_image query_frontend QueryFrontendDockerfile $public
    create_image management_frontend ManagementFrontendDockerfile $public
    create_image unittests ClipperTestsDockerfile  $private
+    create_image unittests ClipperPy3TestsDockerfile  $private


Change the name of the Py3 unittests image to py3tests

AmplabJenkins · 2018-03-09T00:56:04Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1084/
Test FAILed.

AmplabJenkins · 2018-03-09T01:02:31Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1085/
Test FAILed.

AmplabJenkins · 2018-03-09T02:21:14Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1087/
Test FAILed.

AmplabJenkins · 2018-03-09T03:09:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1090/
Test FAILed.

dcrankshaw

This is super close! I added a comment that should help get the tests passing, and then this will be good to go.

dcrankshaw · 2018-03-12T19:29:07Z

.pypirc

+[test]
+repository = https://test.pypi.org/legacy/
+username = jwirjo
+password = clippertest


You definitely don't want to commit this!

dcrankshaw · 2018-03-12T19:30:04Z

bin/run_unittests.sh

@@ -197,7 +198,7 @@ case $args in
    -r | --rpc-container )  set_test_environment
                            run_rpc_container_tests
                            ;;
-    -i | --integration_tests )  set_test_environment
+    -i | --integration_tests )  #set_test_environment


revert this change

dcrankshaw · 2018-03-12T19:36:39Z

dockerfiles/Py3ClosureContainerDockerfile

+
+COPY clipper_admin/clipper_admin/python3_container_conda_deps.txt /lib/
+RUN conda install -y --file /lib/python3_container_conda_deps.txt
+RUN conda install -c anaconda3 cloudpickle=0.5.2


Change lines 4-5 to:

RUN conda config --set ssl_verify no \ && conda install -c anaconda3 cloudpickle=0.5.2 \ && conda install -y --file /lib/python3_container_conda_deps.txt

This should fix the current test failure (see #426)

AmplabJenkins · 2018-03-13T03:52:57Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1100/
Test FAILed.

dcrankshaw · 2018-03-13T03:54:50Z

bin/run_unittests.sh

@@ -140,6 +140,7 @@ function run_integration_tests {
  python ../integration-tests/deploy_pyspark_models.py
  python ../integration-tests/deploy_pyspark_pipeline_models.py
  python ../integration-tests/deploy_pyspark_sparkml_models.py
+  python ../integration-tests/deploy_pytorch_models.py


Since this test seems to be failing, you can revert it and I'll add it in a separate PR.

AmplabJenkins · 2018-03-15T07:31:37Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1127/
Test FAILed.

AmplabJenkins · 2018-03-17T05:14:23Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1131/
Test FAILed.

dcrankshaw · 2018-03-17T05:40:43Z

jenkins test this please

AmplabJenkins · 2018-04-10T00:10:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1326/
Test FAILed.

AmplabJenkins · 2018-04-10T02:48:35Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1328/
Test FAILed.

AmplabJenkins · 2018-04-10T15:16:52Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1333/
Test FAILed.

dcrankshaw · 2018-04-10T15:49:29Z

jenkins test this please

AmplabJenkins · 2018-04-10T16:16:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1334/
Test FAILed.

dcrankshaw · 2018-04-24T21:03:45Z

@jwirjo @rohsuresh Any progress on this?

AmplabJenkins · 2018-05-10T22:57:04Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1419/
Test FAILed.

AmplabJenkins · 2018-05-11T01:50:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1421/
Test FAILed.

AmplabJenkins · 2018-05-11T02:04:36Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1423/
Test FAILed.

dcrankshaw · 2018-05-11T02:05:33Z

jenkins test this please

AmplabJenkins · 2018-05-11T02:14:07Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1424/
Test FAILed.

dcrankshaw · 2018-05-11T02:17:55Z

jenkins test this please

AmplabJenkins · 2018-05-11T02:29:08Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1425/
Test FAILed.

AmplabJenkins · 2018-05-11T03:43:17Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1426/
Test FAILed.

AmplabJenkins · 2018-05-11T05:02:35Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1427/
Test PASSed.

AmplabJenkins · 2018-05-11T07:38:15Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/1428/
Test PASSed.

dcrankshaw requested changes Feb 23, 2018

View reviewed changes

dcrankshaw added status: needs revision type: enhancement labels Feb 23, 2018

dcrankshaw mentioned this pull request Feb 23, 2018

Py 3 RPC implementation + Clipper Admin compatibility with Python 3 #409

Closed

dcrankshaw reviewed Feb 24, 2018

View reviewed changes

dcrankshaw changed the title ~~Py2py3updated~~ Add Python 3 support Feb 28, 2018

Py 3 RPC implementation + Clipper Admin compatibility with Python 3

0f5954d

jwirjo and others added 5 commits March 8, 2018 17:03

tests

92aeafb

tests

1c63302

progress

1e6a788

got wheel working and removed build files

b8047cf

progress 2

c9b7799

jwirjo force-pushed the py2py3updated branch from 690d76b to c9b7799 Compare March 9, 2018 01:05

removed deps formatted code

70c77cf

dcrankshaw requested changes Mar 12, 2018

View reviewed changes

Merge branch 'develop' into py2py3updated

a688620

dcrankshaw reviewed Mar 13, 2018

View reviewed changes

remove pytorch test

5f57bd9

address comments

80c7552

changed kube connections generator in integration tests

27e7d51

format code

95912cf

Merge remote-tracking branch 'ucbrise/develop' into py2py3updated

8959109

jwirjo mentioned this pull request Apr 26, 2018

Py3 #482

Closed

dcrankshaw added 4 commits May 10, 2018 17:09

Merge remote-tracking branch 'ucbrise/develop' into py2py3updated

8a1e61e

Python 3 support seems to be fully working

f5a7eee

format code

8174f48

Merge remote-tracking branch 'ucbrise/develop' into py2py3updated

e012e33

format code

1f4f281

comment out some unittests

d5e52f4

force exit

2d0e226

fix integration test

b0d22ce

uncomment tests

bb25ebf

dcrankshaw merged commit 735815f into ucbrise:develop May 11, 2018

		@@ -42,6 +42,7 @@ def save_python_function(name, func):
		c.dump(func)
		serialized_prediction_function = s.getvalue()

		@@ -72,6 +74,7 @@ def create_endpoint(
		:py:meth:`clipper.ClipperConnection.set_num_replicas`.
		"""

		&& git apply ../patches/make_spdlog_compile_linux.patch


		ENTRYPOINT ["/clipper/bin/ci_checks.sh"]

		@@ -0,0 +1,20 @@
		# This ARG isn't used but prevents warnings in the build script

Add Python 3 support #416

Add Python 3 support #416

Conversation

jwirjo commented Feb 21, 2018 • edited by dcrankshaw

AmplabJenkins commented Feb 21, 2018

dcrankshaw commented Feb 23, 2018

dcrankshaw commented Feb 23, 2018

dcrankshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Mar 9, 2018

AmplabJenkins commented Mar 9, 2018

AmplabJenkins commented Mar 9, 2018

AmplabJenkins commented Mar 9, 2018

dcrankshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Mar 13, 2018

Choose a reason for hiding this comment

AmplabJenkins commented Mar 15, 2018

AmplabJenkins commented Mar 17, 2018

dcrankshaw commented Mar 17, 2018

AmplabJenkins commented Apr 10, 2018

AmplabJenkins commented Apr 10, 2018

AmplabJenkins commented Apr 10, 2018

dcrankshaw commented Apr 10, 2018

AmplabJenkins commented Apr 10, 2018

dcrankshaw commented Apr 24, 2018

AmplabJenkins commented May 10, 2018

AmplabJenkins commented May 11, 2018

AmplabJenkins commented May 11, 2018

dcrankshaw commented May 11, 2018

AmplabJenkins commented May 11, 2018

dcrankshaw commented May 11, 2018

AmplabJenkins commented May 11, 2018

AmplabJenkins commented May 11, 2018

AmplabJenkins commented May 11, 2018

AmplabJenkins commented May 11, 2018

jwirjo commented Feb 21, 2018 •

edited by dcrankshaw