New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtualenv support for python actions #1940

Closed
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@RSulzmann
Contributor

RSulzmann commented Mar 3, 2017

Allow a virtualenv directory in a zip file for a python action.

This allows to package python modules installed with pip into the action zip file.
The invoker activates the virtual python environment before invoking the action.

Naming conventions :

  1. The name of the main python module in the zip file must be : __main__.py
  2. The directory name of the virtual environment must be : virtualenv

How to create a zip file with a virtual environment:

  1. name your python file __main__.py
  2. install virtualenv: (pip install virtualenv)
  3. create the virtual environment named 'virtualenv': virtualenv virtualenv
  4. activate the virtual environment: source virtualenv/bin/activate
  5. install all required modules (e.g. pip install xxx)
  6. deactivate the virtual environment: deactivate
  7. zip the files: zip -r myPython.zip virtualenv __main__.py
  8. create action: wsk action create actionname --kind python myPython.zip
@markusthoemmes

This comment has been minimized.

Contributor

markusthoemmes commented Mar 3, 2017

Add a test?
Add documentation?

# activate the virtualenv
activate_this_file = path_to_virtualenv + '/bin/activate_this.py'
if os.path.exists(activate_this_file):
execfile(activate_this_file, dict(__file__=activate_this_file))

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 3, 2017

Contributor

Do we need to define this twice? Does the first parameter suffice?

This comment has been minimized.

@RSulzmann

RSulzmann Mar 17, 2017

Contributor

execfile does not modify the globals (as file) so the executed script may take an incorrect path.
You can pass global variables to execfile so you can modify its file variable

@jeremiaswerner

This comment has been minimized.

Contributor

jeremiaswerner commented Mar 3, 2017

works nice 👍

(minor: when I played through I saw the action must be named __main__.py instead of main.py)

@rabbah

This comment has been minimized.

Member

rabbah commented Mar 3, 2017

good to see this.
if the zip includes native (compiled) libraries, did you verify if they will work?
we need tests, of course: please see PythonActionContainerTests and CLIPythonTests.

4. activate the virtual environment: `source virtualenv/bin/activate`
5. install all required modules `e.g. pip install xxx`
6. deactivate the virtual environment: `deactivate`
7. zip the files: `zip -r helloPython.zip virtualenv __main__.py`

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 6, 2017

Contributor

Should we automate this process? Could be

virtualenv virtualenv
source virtualenv/bin/activate
pip install -r requirements.txt
zip -r helloPython.zip *
@cclauss

This comment has been minimized.

Contributor

cclauss commented Mar 6, 2017

How do you folks feel about Python 2 vs. Python 3? In Python 2, virtualenv is the way to go but since Python 3.3 venv has been a module in the standard library and since Python 3.5 "venv is now recommended for creating virtual environments.".

$ python3 -m venv . ; . bin/activate

I know that OpenWhisk is Python 2 currently but I have been submitting PRs to get us to compatibility with both Py2 and Py3. I provide some of the reasons for this in #1899 and it has been suggested in #1184 (comment) that we only support one of the two Python versions.

@rabbah

This comment has been minimized.

Member

rabbah commented Mar 6, 2017

@cclauss good points - we need to accelerate moving to python 3/adding support so all new changes at in the newer domain. on this note, i think once we have python 3, we can mark python 2 deprecated and eventually remove it.

@rabbah

This comment has been minimized.

Member

rabbah commented Mar 7, 2017

The docs should be updated to show that you can just zip python files without a venv as well (the simpler case).

@cclauss

This comment has been minimized.

Contributor

cclauss commented Mar 7, 2017

Simpler but less powerful... With this technique you can bring along third party modules that are not already included in the Docker container.

@rabbah

This comment has been minimized.

Member

rabbah commented Mar 7, 2017

Docs should not preclude the simpler use case which is easier for tire kickers.

@@ -0,0 +1,126 @@
/*

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 11, 2017

Contributor

Rename CLIPythonTests.scala to WskBasicPythonTests.scala and put these tests in there as well?

You can package a Python action and dependant modules in a zip file.
The virtual environment package *virtualenv* is supported to pass pip installed packages together with the action in the zip file.
Package installations inside virtualenv must be done in the target environment (alpine, python 2.7), so a docker image (e.g. jfloff/alpine-python:2.7) is used to create the virtualenv directory.

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 11, 2017

Contributor

Why not use the official python image that we use here?

This comment has been minimized.

@cclauss

cclauss Mar 11, 2017

Contributor

I believe this approach is putting more power into the hands of the action developer. She is not limited to the pip modules that are in the docker image but can bring along other pip modules (or newer/older versions) with her code. In this scenario, she would:

  1. Make a virtual environment
  2. Pip install her favorite modules
  3. Add her code
  4. Test the whole thing locally
  5. Create an OpenWhisk action that bundles her exact environment

This comment has been minimized.

@cclauss

cclauss Mar 11, 2017

Contributor

Oh, now I see that you were reacting to jfloff/alpine-python:2.7. I agree, it would be safer to use the OpenWhisk pythonAction image.

Creating the zip file:
1. name your main Python file `__main__.py`
2. Change into docker image: `docker run -it -v "$PWD:/tmp" jfloff/alpine-python:2.7 bash`

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 11, 2017

Contributor

Here as well

6. activate the virtual environment: `source virtualenv/bin/activate`
7. install all required modules `e.g. pip install xxx`
8. deactivate the virtual environment: `deactivate`
9. exit docker image: `exit`

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 11, 2017

Contributor

Why don't we do all these steps in one single command? Advocate the use of requirements.txt and then do:

docker run -v "$PWD:/tmp" python:2.7.13-alpine sh -c "pip install virtualenv && cd /tmp && virtualenv virtualenv && source virtualenv/bin/activate && pip install -r requirements.txt"

Should be perfectly fine and far less manual steps.

This comment has been minimized.

@cclauss

cclauss Mar 11, 2017

Contributor

Advocating the use requirements.txt is a great idea. Somewhere between step 7 and 8, the action developer is going to need to add her code and test out her code within the virtualenv.

@@ -21,6 +21,7 @@ RUN pip install \
requests==2.11.1 \
scrapy==1.1.2 \
simplejson==3.8.2 \
virtualenv==15.1.0 \

This comment has been minimized.

@markusthoemmes

markusthoemmes Mar 11, 2017

Contributor

Document the added package.

cclauss added a commit to cclauss/openwhisk that referenced this pull request Mar 11, 2017

Added virtualenv
In sympathy with apache#1940

cclauss added a commit to cclauss/openwhisk that referenced this pull request Mar 11, 2017

Added virtualenv
In sympathy with apache#1940

cclauss added a commit to cclauss/openwhisk that referenced this pull request Mar 11, 2017

Added virtualenv
In sympathy with apache#1940
implicit val wskprops = WskProps()
val wsk = new Wsk
val zippedPythonAction = Some(TestUtils.getTestActionFilename("action_python_virtualenv.zip"))

This comment has been minimized.

@rabbah

rabbah Mar 18, 2017

Member

i would prefer if these tests are primarily unit tests (in PythonActionContainerTests instead). The CLI tests should then test only that the overall integration works for one example. Unit tests are easier to debug, and separate concerns.

@jthomas

This comment has been minimized.

Member

jthomas commented Mar 18, 2017

This will make it super easy to support Python actions in the Serverless Framework plugin. Good work 👏.

withActivation(wsk.activation, wsk.action.invoke(name)) {
activation =>
val response = activation.response
System.out.println("ROBI:" + response.result.get.fields)

This comment has been minimized.

@jeremiaswerner

jeremiaswerner Mar 20, 2017

Contributor

remove this?

This comment has been minimized.

@RSulzmann

RSulzmann Mar 20, 2017

Contributor

done

withActivation(wsk.activation, wsk.action.invoke(name)) {
activation =>
val response = activation.response
System.out.println("ROBI:" + response.result.get.fields)

This comment has been minimized.

@jeremiaswerner

jeremiaswerner Mar 20, 2017

Contributor

remove this?

This comment has been minimized.

@RSulzmann

RSulzmann Mar 20, 2017

Contributor

done

withActivation(wsk.activation, wsk.action.invoke(name)) {
activation =>
val response = activation.response
System.out.println("ROBI:" + response.result.get.fields)

This comment has been minimized.

@jeremiaswerner

jeremiaswerner Mar 20, 2017

Contributor

remove this?

This comment has been minimized.

@RSulzmann

RSulzmann Mar 20, 2017

Contributor

done

@@ -68,7 +71,7 @@ class CLIPythonTests
(wp, assetHelper) =>
val name = "pythonZipWithNonDefaultEntryPoint"
assetHelper.withCleaner(wsk.action, name) {
(action, _) => action.create(name, Some(TestUtils.getTestActionFilename("python.zip")), main = Some("niam"), kind = Some("python:default"))
(action, _) => action.create(name, Some(TestUtils.getTestActionFilename("python.zip")), main = Some("niam"), kind = Some("python"))

This comment has been minimized.

@rabbah

rabbah Mar 30, 2017

Member

is this deliberate? you want to run with python 2 not 3?

@rabbah rabbah self-assigned this Mar 30, 2017

### Packaging Python actions with a virtual environment in zip files
The virtual environment package *virtualenv* is supported to pass pip installed packages together with the action in the zip file.
To ensure compatibility with the openwhisk container, package installations inside virtualenv must be done in the target environment.

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

openwhisk -> OpenWhisk

This comment has been minimized.

@RSulzmann

RSulzmann Apr 3, 2017

Contributor

done

Creating the zip file:
1. Name your main Python file `__main__.py`

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

1 is already stated above

This comment has been minimized.

@RSulzmann

RSulzmann Apr 3, 2017

Contributor

line removed

This comment has been minimized.

@rabbah

rabbah Apr 3, 2017

Member

did you push the latest changes?

$ docker run -v "$PWD:/tmp" openwhisk/pythonaction sh -c "cd tmp; virtualenv virtualenv; source virtualenv/bin/activate; pip install -r requirements.txt;"
```
4. Zip the virtualenv directory and your Python file(s): `$ zip -r helloPython.zip virtualenv __main__.py`

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

keep this in triple back ticks: easier to copy/paste an entire line that way

This comment has been minimized.

@RSulzmann

RSulzmann Apr 3, 2017

Contributor

done

}
it should "report error if zipped Python action has wrong main module name" in {
val zippedPythonActionWrongName = TestUtils.getTestActionFilename("action_python_virtualenv_name.zip")

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

could you have reused the same zip file as above but passed main = "foobar" below?
ah this is testing a missing __main__.py.

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

is there a positive test for using a non-standard "main" entry point with venv?

checkStreams(out, err, {
case (o, e) =>
o shouldBe empty
e should include("Exec format error")

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

is this an error you can easily catch and rewrite to a more elaborate error message?

}
}
it should "Ensure that zipped Python actions cannot be created without a kind specified" in withAssetCleaner(wskprops) {

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

@dubeejw this is a test we should have in basic usage tests - is there one already?
if so, i would remove this test from this suite.
if not, i would move this test to WskBasicUsageTests.

behavior of "Python runtime"
it should "invoke a zipped Python action with virtualenv package" in withAssetCleaner(wskprops) {

This comment has been minimized.

@rabbah

rabbah Apr 1, 2017

Member

will this work with python:3?
have you tried?
if it does, great - i suspect it will because the python action unit tests run for both runtimes, so if you have a clean build, it will.
if it does not, the documentation should reflect the restriction.

This comment has been minimized.

@RSulzmann

RSulzmann Apr 4, 2017

Contributor

works with python3 using a zipped virtualenv that is built with python3
added the appropriate python3 test zip
will update documentation

@rabbah

rabbah requested changes Apr 1, 2017 edited

see comments.
i didn't see a pg tagged. was there one?

@cclauss

This comment has been minimized.

Contributor

cclauss commented Apr 2, 2017

I would still be interested in a discussion on the use of the builtin venv vs virtualenv now that Python 3 is the default.

@csantanapr

This comment has been minimized.

Contributor

csantanapr commented Apr 3, 2017

@cclauss

I would still be interested in a discussion on the use of the builtin venv vs virtualenv now that Python 3 is the default.

python 2 is the default

@cclauss

This comment has been minimized.

Contributor

cclauss commented Apr 3, 2017

You are correct. I missed the changes made in #2094 Bummer. Given the stance of Python.org, we should reopen #1899

}
if (imageName == "python3action") {
zippedPythonActionName = TestUtils.getTestActionFilename("action_python3_virtualenv.zip")
}

This comment has been minimized.

@markusthoemmes

markusthoemmes Apr 4, 2017

Contributor

You can use a scala idiomatic way to no use a variable (mutability is to be carefully chosen and actually not needed here).

In scala if is an expression, which means it does return a value from each branch instead of just executing it. With that in mind, you can rewrite your code to:

val zippedPythonActionName = if (imageName == "python2action") "action_python2_virtualenv.zip" else "action_python3_virtualenv.zip"
val zippedPythonAction = TestUtils.getTestActionFilename(zippedPythonActionName)

Does that make sense?

@@ -212,25 +217,6 @@ class PythonActionContainerTests extends BasicActionRunnerTests with WskActorSys
})
}
it should "report error if zipped Python action has virtualenv created in wrong environment" in {

This comment has been minimized.

@markusthoemmes

markusthoemmes Apr 4, 2017

Contributor

Why is this test removed?

This comment has been minimized.

@RSulzmann

RSulzmann Apr 4, 2017

Contributor

back and changed to support python2 and python3 environments
testcase: try to run python2 virtualenv in python3 container and vice versa

```
$ docker run -v "$PWD:/tmp" openwhisk/pythonaction sh -c "cd tmp; virtualenv virtualenv; source virtualenv/bin/activate; pip install -r requirements.txt;"
$ docker run -v "$PWD:/tmp" openwhisk/python3action sh -c "cd tmp; virtualenv virtualenv; source virtualenv/bin/activate; pip install -r requirements.txt;"

This comment has been minimized.

@markusthoemmes

markusthoemmes Apr 4, 2017

Contributor

Add --rm to the command so the user doesn't manually need to remove the container afterwards.

This comment has been minimized.

@RSulzmann

RSulzmann Apr 4, 2017

Contributor

done

So the docker image *openwhisk/pythonaction* is used to create the virtualenv directory.
The virtual environment package `virtualenv` is supported to pass pip installed packages together with the action in the zip file.
To ensure compatibility with the OpenWhisk container, package installations inside virtualenv must be done in the target environment.
So the docker image `openwhisk/pythonaction` is used to create the virtualenv directory.

This comment has been minimized.

@markusthoemmes

markusthoemmes Apr 4, 2017

Contributor

Adjust to reflect that you're using either openwhisk/python{2,3}Action respectively.

This comment has been minimized.

@RSulzmann

RSulzmann Apr 4, 2017

Contributor

done

@RSulzmann

This comment has been minimized.

Contributor

RSulzmann commented Apr 4, 2017

Playground PG1 #1358

@RSulzmann

This comment has been minimized.

Contributor

RSulzmann commented Apr 5, 2017

Playground PG1 #1360

@rabbah

rabbah approved these changes Apr 7, 2017

@rabbah

This comment has been minimized.

Member

rabbah commented Apr 7, 2017

PG2/1415 for the latest revisions.

@rabbah

This comment has been minimized.

Member

rabbah commented Apr 8, 2017

Squashed and merged as d3f4f95.

@rabbah rabbah closed this Apr 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment