search

openfaas / python-flask-template

HTTP and Flask-based OpenFaaS templates for Python 3
MIT License
85 stars 86 forks source link

Create debian templates for the python 3 flask templates. #26

Closed Jeff-Lowrey closed 4 years ago

Jeff-Lowrey commented 4 years ago

The current python3-flask templates use the Alpine base python image. Precompiled wheels for many important python libraries are not available for Alpine, but are easily available for Debian. Using Debian makes it much simpler and easier to use python packages like Numpy or Pillow.

Signed-off-by: Jeff Lowrey jefferson.lowrey@mantech.com

LucasRoesler commented 4 years ago

@Jeff-Lowrey i am sorry this took me so long to get to, but I just tried building the python3-flask-debian template and it failed to install gevent

15 9.977     copying src/gevent/tests/monkey_package/script.py -> build/lib.linux-x86_64-3.8/gevent/tests/monkey_package
#15 9.977     copying src/gevent/libuv/_corecffi_cdef.c -> build/lib.linux-x86_64-3.8/gevent/libuv
#15 9.977     copying src/gevent/libuv/_corecffi_source.c -> build/lib.linux-x86_64-3.8/gevent/libuv
#15 9.977     running build_ext
#15 9.977     Running '(cd  "/tmp/pip-install-q4j03vrh/gevent/deps/libev"  && sh ./configure   && cp config.h "$OLDPWD" ) > configure-output.txt' in /tmp/pip-install-q4j03vrh/gevent/build/temp.linux-x86_64-3.8/libev
#15 9.977     configure: error: in `/tmp/pip-install-q4j03vrh/gevent/deps/libev':
#15 9.977     configure: error: no acceptable C compiler found in $PATH
#15 9.977     See `config.log' for more details
#15 9.977     Traceback (most recent call last):
#15 9.977       File "<string>", line 1, in <module>
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/setup.py", line 427, in <module>
#15 9.977         run_setup(EXT_MODULES, run_make=_BUILDING)
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/setup.py", line 328, in run_setup
#15 9.977         setup(
#15 9.977       File "/usr/local/lib/python3.8/site-packages/setuptools/__init__.py", line 144, in setup
#15 9.977         return distutils.core.setup(**attrs)
#15 9.977       File "/usr/local/lib/python3.8/distutils/core.py", line 148, in setup
#15 9.977         dist.run_commands()
#15 9.977       File "/usr/local/lib/python3.8/distutils/dist.py", line 966, in run_commands
#15 9.977         self.run_command(cmd)
#15 9.977       File "/usr/local/lib/python3.8/distutils/dist.py", line 985, in run_command
#15 9.977         cmd_obj.run()
#15 9.977       File "/usr/local/lib/python3.8/site-packages/setuptools/command/install.py", line 61, in run
#15 9.977         return orig.install.run(self)
#15 9.977       File "/usr/local/lib/python3.8/distutils/command/install.py", line 545, in run
#15 9.977         self.run_command('build')
#15 9.977       File "/usr/local/lib/python3.8/distutils/cmd.py", line 313, in run_command
#15 9.977         self.distribution.run_command(command)
#15 9.977       File "/usr/local/lib/python3.8/distutils/dist.py", line 985, in run_command
#15 9.977         cmd_obj.run()
#15 9.977       File "/usr/local/lib/python3.8/distutils/command/build.py", line 135, in run
#15 9.977         self.run_command(cmd_name)
#15 9.977       File "/usr/local/lib/python3.8/distutils/cmd.py", line 313, in run_command
#15 9.977         self.distribution.run_command(command)
#15 9.977       File "/usr/local/lib/python3.8/distutils/dist.py", line 985, in run_command
#15 9.977         cmd_obj.run()
#15 9.977       File "/usr/local/lib/python3.8/site-packages/setuptools/command/build_ext.py", line 87, in run
#15 9.977         _build_ext.run(self)
#15 9.977       File "/usr/local/lib/python3.8/distutils/command/build_ext.py", line 340, in run
#15 9.977         self.build_extensions()
#15 9.977       File "/usr/local/lib/python3.8/distutils/command/build_ext.py", line 449, in build_extensions
#15 9.977         self._build_extensions_serial()
#15 9.977       File "/usr/local/lib/python3.8/distutils/command/build_ext.py", line 474, in _build_extensions_serial
#15 9.977         self.build_extension(ext)
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/_setuputils.py", line 222, in build_extension
#15 9.977         self.gevent_prepare(ext)
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/_setuputils.py", line 219, in gevent_prepare
#15 9.977         configure(self, ext)
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/_setuplibev.py", line 56, in configure_libev
#15 9.977         system(libev_configure_command)
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/_setuputils.py", line 150, in system
#15 9.977         if _system(cmd, cwd=cwd, env=env, **kwargs):
#15 9.977       File "/tmp/pip-install-q4j03vrh/gevent/_setuputils.py", line 146, in _system
#15 9.977         return check_call(cmd, cwd=cwd, env=env, **kwargs)
#15 9.977       File "/usr/local/lib/python3.8/subprocess.py", line 364, in check_call
#15 9.977         raise CalledProcessError(retcode, cmd)
#15 9.977     subprocess.CalledProcessError: Command '(cd  "/tmp/pip-install-q4j03vrh/gevent/deps/libev"  && sh ./configure   && cp config.h "$OLDPWD" ) > configure-output.txt' returned non-zero exit status 1.
#15 9.977     ----------------------------------------
#15 10.04 ERROR: Command errored out with exit status 1: /usr/local/bin/python -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-q4j03vrh/gevent/setup.py'"'"'; __file__='"'"'/tmp/pip-install-q4j03vrh/gevent/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /tmp/pip-record-t39nms9c/install-record.txt --single-version-externally-managed --compile --install-headers /usr/local/include/python3.8/gevent Check the logs for full command output.
#15 ERROR: executor failed running [/bin/sh -c pip install -r requirements.txt]: runc did not terminate sucessfully

it seems like the require gcc is missing

Jeff-Lowrey commented 4 years ago

@LucasRoesler I wasn't sure if that was included in the debian slim or not, so I left it out. Obviously, it's not. The three packages that are installed directly by the docker file are musl-dev, gcc, and make. I'll go back and add these in. I do remember some issues installing at least one of them - the musl-dev I believe - as it was already there.

Maybe they should be put into the requirements.txt file, though?

LucasRoesler commented 4 years ago

@Jeff-Lowrey the template should compile as is without any extra changes from the user. The base template is currently broken, the user would have to effectively fix it before they could do anything. If the template is going to use gevent to run the python server, then it should include all of the dependencies to make it run

All of the official templates will build into "echo" functions if the user does absolutely nothing. These should behave the same way.

When i built my debian slim image for python, I most likely didn't run in to this because i used conda https://github.com/LucasRoesler/pydatascience-template/blob/master/template/pydatascience-web/Dockerfile

Jeff-Lowrey commented 4 years ago

@LucasRoesler I completely agree about the templates working "as is" - I just didn't test the python3-flask-debian template.

I'm trying to modify the existing python3-flask/http templates as little as possible, so that's why I'm not switching to using Conda.

I thought that moving the musl-lib/gcc/make dependencies into requirements.txt would be a nice optimization, but it's actually too late in the build to do that.

I am able to fix the dockerfile so that it includes musl-lib, gcc, and make so that gevent will compile and install without changes from user.

But I do wonder why the python3-flask template uses gevent and the python3-http template uses waitress...

@alexellis Does the gevent WSGI server provide better/different performance than the waitress server?

I see that the index.py for each does different things, and passes different parameters to the handler functions. Does the flask-template give more control at the http request/response level?

Jeff-Lowrey commented 4 years ago

I think we should move further conversation on this PR to https://openfaas.slack.com/archives/C72R8LGN6/p1584972575236200

But apparently it's better to have the conversation here, so scratch that.

Consensus is that having discussion here is the right place for it, ergo we go ahead with that.

@alexellis @LucasRoesler What are the motivating factors for the flask template(s) using gevent and the http template(s) using waitress? Performance? better control of the data vs. more dev-friendly interface? evolution over time?

LucasRoesler commented 4 years ago

@Jeff-Lowrey I don't really know the history behind that. the original move to gevent happened here https://github.com/openfaas-incubator/python-flask-template/commit/cf65d0a33ab4c139889e6549e446b24d39987e0e

Most importantly to note is

gevent is a more suitable framework for high performance throughput and benchmarking. This performs around 2x faster than Flask with debug=False when tested on loopback with hey on Kube

To the point made by Blaise image

waitress is a pure python wsgi server, so it is easier to install on Alpine and provides a slight boost to the performance.

I have tried to find some comparisons, but nothing jumped up in my google search immediately. But i think it is ok for us to consider either (1) doing a little research and standardizing or (2) continue using gevent and waitress as appropriate to the environment

Jeff-Lowrey commented 4 years ago

A couple of additional angles to look at:

This is what I think

Another feature request (for me at least) will be to make it easier to do more advanced work with the response message. One of the things I'm struggling with a bit right now is how to return a multi-part message. Both the flask and the http templates make that a bit complicated.

blaisep commented 4 years ago

@LucasRoesler ,

I have tried to find some comparisons, but nothing jumped up in my google search immediately. But i think it is ok for us to consider either (1) doing a little research and standardizing or (2) continue using gevent and waitress as appropriate to the environment

I will bring this up in the PyBites slack channel. There is a broad spectrum of hard core Python gearheads.

Jeff-Lowrey commented 4 years ago

@blaisep I'm not initially trying to optimize for anything, just get a couple of templates available that use Debian. I needed to use a couple of libraries (Pillow and the cryptography package) that needed to be compiled on Alpine, but didn't need to be compiled on Debian.

So it was a lot easier for me to switch to using a Debian based image than throw in a lot of stuff into an Alpine based image. And there was a similar issue from Ziyun Zhao on the Slack channel, so it seemed like a good idea to add a Debian option to this set of templates.

Other issues/requests may be raised to address other parts of this overall picture.

LucasRoesler commented 4 years ago

I don't mean picking Alpine over debian, I mean using gevent in or waitress depending on what we can/are willing to maintain

Jeff-Lowrey commented 4 years ago

@LucasRoesler Yes, this PR is just for adding a Debian alternative. Not for replacing alpine with Debian, nor for choosing between waitress and gevent.

A separate choice to only support Debian might be made. But that's outside of what I'm doing here.

Also, I'm doing some cleanup and expect to have a new commit ready for testing soon.

blaisep commented 4 years ago

OK, upon careful re-reading, I came to the awkward conclusion that I didn't make a useful contribution to the discussion. I will go ahead and delete my comments because they are sort of orthogonal to the PR.

Jeff-Lowrey commented 4 years ago

I neatened up and commented the docker files (but not for the 27 and armhf ones... ) and verified that my new debian templates will build.

@LucasRoesler can you test again?

If the comments in the docker files look good, I'll update the remainder.

LucasRoesler commented 4 years ago

@Jeff-Lowrey it builds now, but .... it doesn't run TypeError: an integer is required (got type bytes)

➜  debiantest docker run --rm -p 8080:8080 tester:latest
Forking - python [index.py]
2020/03/28 09:29:01 Started logging stderr from function.
2020/03/28 09:29:01 Started logging stdout from function.
2020/03/28 09:29:01 OperationalMode: http
2020/03/28 09:29:01 Timeouts: read: 10s, write: 10s hard: 10s.
2020/03/28 09:29:01 Listening on port: 8080
2020/03/28 09:29:01 Writing lock-file to: /tmp/.lock
2020/03/28 09:29:01 Metrics listening on port: 8081
2020/03/28 09:29:01 stderr: Traceback (most recent call last):
2020/03/28 09:29:01 stderr:   File "index.py", line 33, in <module>
2020/03/28 09:29:01 stderr:     http_server.serve_forever()
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/baseserver.py", line 367, in serve_forever
2020/03/28 09:29:01 stderr:     self.start()
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/baseserver.py", line 305, in start
2020/03/28 09:29:01 stderr:     self.init_socket()
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/pywsgi.py", line 1491, in init_socket
2020/03/28 09:29:01 stderr:     self.update_environ()
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/pywsgi.py", line 1503, in update_environ
2020/03/28 09:29:01 stderr:     name = socket.getfqdn(address[0])
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_socketcommon.py", line 269, in getfqdn
2020/03/28 09:29:01 stderr:     hostname, aliases, _ = gethostbyaddr(name)
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_socketcommon.py", line 241, in gethostbyaddr
2020/03/28 09:29:01 stderr:     return get_hub().resolver.gethostbyaddr(ip_address)
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/resolver/thread.py", line 68, in gethostbyaddr
2020/03/28 09:29:01 stderr:     return self.pool.apply(_socket.gethostbyaddr, args, kwargs)
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/pool.py", line 159, in apply
2020/03/28 09:29:01 stderr:     return self.spawn(func, *args, **kwds).get()
2020/03/28 09:29:01 stderr:   File "src/gevent/event.py", line 268, in gevent._event.AsyncResult.get
2020/03/28 09:29:01 stderr:   File "src/gevent/event.py", line 296, in gevent._event.AsyncResult.get
2020/03/28 09:29:01 stderr:   File "src/gevent/event.py", line 286, in gevent._event.AsyncResult.get
2020/03/28 09:29:01 stderr:   File "src/gevent/event.py", line 266, in gevent._event.AsyncResult._raise_exception
2020/03/28 09:29:01 stderr:   File "src/gevent/event.py", line 211, in gevent._event.AsyncResult.exc_info.__get__
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_tblib.py", line 371, in g
2020/03/28 09:29:01 stderr:     return f(a)
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_tblib.py", line 432, in load_traceback
2020/03/28 09:29:01 stderr:     return loads(s)
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_tblib.py", line 337, in unpickle_traceback
2020/03/28 09:29:01 stderr:     return ret.as_traceback()
2020/03/28 09:29:01 stderr:   File "/usr/local/lib/python3.8/site-packages/gevent/_tblib.py", line 202, in as_traceback
2020/03/28 09:29:01 stderr:     code = CodeType(
2020/03/28 09:29:01 stderr: TypeError: an integer is required (got type bytes)
2020/03/28 09:29:01 Forked function has terminated: exit status 1
Jeff-Lowrey commented 4 years ago

Taking all of these notes into consideration, some comments:

  1. I didn't go through the python3-http template.yml file, and obviously should have. It's the same as the one in the alpine version. So pruning a bunch of things is a good idea. .
  2. The of-watchdog version was also pulled from the existing python3 templates.
  3. Reviewing the man page for apt-get, using -qy is the much better option, quiet to limit output and -y to assume yes. --assume-yes was pulled from other places.
  4. I didn't set a minor version on the python level because at least some of the stuff I'm using is 3.8, not 3.7. The rest of the templates are at 3.7, so it makes sense to use that, except it probably leaves me in the same place of having to use customized templates locally. But I also don't know how often templates get pulled against existing functions? Maybe that happens all the time with the openfaas-cloud. If it doesn't happen all the time, then is there a strong reason not to upgrade all the templates to 3.8?

I'll go through and prune the libs in the template.yml file in use by my function that needs Pillow and cryptography and see if I can get a clean separation between what's needed for those specific libs and what's needed for the base template.

I'm not entirely sure that anything other than gcc, make, and git are needed. I'm really confused that subversion is in there at all - again, that comes from the existing python3-http template.

alexellis commented 4 years ago

@Jeff-Lowrey when could you fix up the issues we have discussed here and send an updated commit up into this PR?

We are tracking interest and want to release it as soon as we can, for instance see #29.

Alex

Jeff-Lowrey commented 4 years ago

@alexellis I don't know if you got a notification, but I did just push a new commit.

Jeff-Lowrey commented 4 years ago

Okay, I've committed the change to use 0.0.0.0. instead of ''.

I want to be clear here, though. This PR is only about providing debian based templates. It's not about making any updates or changes to the existing templates - those should be addressed in different issues/PRs.

This includes things like:

  1. Upgrading the version of of-watchdog
  2. Addressing the issues mentioned in https://github.com/openfaas/templates/blob/master/template/python3/Dockerfile#L25
  3. normalizing on either waitress or gevent for all the flask templates.
  4. any other changes that affect the other templates in this package.

Decisions on how to address those should be made, and I know that I at least will need a supported version of these templates for item 2 above. But it's not in scope for this PR.