Add Python 3 support by jwirjo · Pull Request #416 · ucbrise/clipper

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

    c.dump(func)
    serialized_prediction_function = s.getvalue()

+


Revert this change

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

-        base_image="clipper/python-closure-container:{}".format(__version__),
+        base_image="default",
        num_replicas=1):
+


Delete extra line

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

        :py:meth:`clipper.ClipperConnection.set_num_replicas`.
    """

+


Delete extra line

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

+                    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

+        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

        return [str(sum(item)) for item in inputs]

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


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

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

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


The Dockerfile file name should not end in .txt

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

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

+    #'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

    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

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

                            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

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

  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

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

Conversation

jwirjo commented Feb 21, 2018 • edited by dcrankshaw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented Feb 21, 2018

Uh oh!

dcrankshaw commented Feb 23, 2018

Uh oh!

dcrankshaw commented Feb 23, 2018

Uh oh!

dcrankshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Mar 9, 2018

Uh oh!

AmplabJenkins commented Mar 9, 2018

Uh oh!

AmplabJenkins commented Mar 9, 2018

Uh oh!

AmplabJenkins commented Mar 9, 2018

Uh oh!

dcrankshaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Mar 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Mar 15, 2018

Uh oh!

AmplabJenkins commented Mar 17, 2018

Uh oh!

dcrankshaw commented Mar 17, 2018

Uh oh!

AmplabJenkins commented Apr 10, 2018

Uh oh!

AmplabJenkins commented Apr 10, 2018

Uh oh!

AmplabJenkins commented Apr 10, 2018

Uh oh!

dcrankshaw commented Apr 10, 2018

Uh oh!

AmplabJenkins commented Apr 10, 2018

Uh oh!

dcrankshaw commented Apr 24, 2018

Uh oh!

AmplabJenkins commented May 10, 2018

Uh oh!

AmplabJenkins commented May 11, 2018

Uh oh!

AmplabJenkins commented May 11, 2018

Uh oh!

jwirjo commented Feb 21, 2018 •

edited by dcrankshaw

Loading