Skip to content
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

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

Closed
wants to merge 5 commits into from
Closed

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

wants to merge 5 commits into from

Conversation

Jeff-Lowrey
Copy link

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 [email protected]

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 <[email protected]>
@LucasRoesler
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Author

Jeff-Lowrey commented Mar 23, 2020

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
Copy link
Member

@Jeff-Lowrey I don't really know the history behind that. the original move to gevent happened here cf65d0a

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
Copy link
Author

Jeff-Lowrey commented Mar 24, 2020

A couple of additional angles to look at:

  • gevent does require more work/additional tools and libraries to use.
  • the gevent based (python3-flask) template provides a "rawer" view of the request. It only provides a string of the request data without any access to the headers or other meta-data about the call. (as far as I can tell) It's not clear to me that even the path or method are visible inside the handler function.
  • the waitress based (python3-http) template does more pre and post processing of the request/response that gives an easier and more complete view of the request and more straight forward creation of the response.

This is what I think

  • For the purposes of this PR, the two templates should be left alone other than being made available based on debian
  • I should open a feature request of some kind (is that what we're calling them) to address the gevent/waitress issue

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
Copy link

blaisep commented Mar 24, 2020

@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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

Jeff-Lowrey commented Mar 24, 2020

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

blaisep commented Mar 24, 2020

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
Copy link
Author

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
Copy link
Member

@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

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)

RUN apt-get update && apt-get --assume-yes install musl-dev gcc make ${ADDITIONAL_PACKAGE}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we adding musl-dev? The whole point of using Debian is to avoid musl and to benefit from all the packages built with glibc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More used to seeing -qy, how does --assume-yes differ?

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)

RUN apt-get update && apt-get --assume-yes install musl-dev gcc make ${ADDITIONAL_PACKAGE}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should install musl-dev 🤔

- g++
- subversion
- python3-dev
- musl-dev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove please

- subversion
- python3-dev
- musl-dev
- libffi-dev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's that package for?

RUN chmod +x /usr/bin/fwatchdog

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this comment now

@@ -0,0 +1,56 @@
FROM openfaas/of-watchdog:0.7.2 as watchdog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the latest version?

@@ -0,0 +1,56 @@
FROM openfaas/of-watchdog:0.7.2 as watchdog
FROM python:3-slim-buster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we should pin a specific Python 3 version, if not, folks will run into issues with libraries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Specific major version)

@Jeff-Lowrey
Copy link
Author

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
Copy link
Member

@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

@alexellis alexellis mentioned this pull request Apr 2, 2020
The template yaml file for the python-http-debian was modified to remove
 unneeded libraries.

Both docker files were changed to force python 3.7.
  Gevent 1.4 is incompatible with Python 3.8, and the rest of the docker
  files use python 3.7.

The apt-get commands were changed from using --assume-yes to -qy. It's shorter
  and adds the --quiet (-q) option to reduce information in stdout during
  docker build.

This has been tested with a bare bones basic function and builds, runs, and
  executes properly.

Signed-off-by: Jeff Lowrey <[email protected]>
@Jeff-Lowrey
Copy link
Author

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

if __name__ == '__main__':
#app.run(host='0.0.0.0', port=5000, debug=False)

http_server = WSGIServer(('', 5000), app)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use

Suggested change
http_server = WSGIServer(('', 5000), app)
http_server = WSGIServer(('0.0.0.0', 5000), app)

binding to 0.0.0.0 resolves this error

Forking - python [index.py]
2020/04/04 14:15:13 Started logging stderr from function.
2020/04/04 14:15:13 Started logging stdout from function.
2020/04/04 14:15:13 OperationalMode: http
2020/04/04 14:15:13 Timeouts: read: 10s, write: 10s hard: 10s.
2020/04/04 14:15:13 Listening on port: 8080
2020/04/04 14:15:13 Writing lock-file to: /tmp/.lock
2020/04/04 14:15:13 Metrics listening on port: 8081
2020/04/04 14:15:13 stderr: Traceback (most recent call last):
2020/04/04 14:15:13 stderr:   File "index.py", line 33, in <module>
2020/04/04 14:15:13 stderr:     http_server.serve_forever()
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/baseserver.py", line 367, in serve_forever
2020/04/04 14:15:13 stderr:     self.start()
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/baseserver.py", line 305, in start
2020/04/04 14:15:13 stderr:     self.init_socket()
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/pywsgi.py", line 1490, in init_socket
2020/04/04 14:15:13 stderr:     StreamServer.init_socket(self)
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/server.py", line 146, in init_socket
2020/04/04 14:15:13 stderr:     self.socket = self.get_listener(self.address, self.backlog, self.family)
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/server.py", line 157, in get_listener
2020/04/04 14:15:13 stderr:     return _tcp_listener(address, backlog=backlog, reuse_addr=cls.reuse_addr, family=family)
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/server.py", line 252, in _tcp_listener
2020/04/04 14:15:13 stderr:     sock = GeventSocket(family=family)
2020/04/04 14:15:13 stderr:   File "/usr/local/lib/python3.7/site-packages/gevent/_socket3.py", line 114, in __init__
2020/04/04 14:15:13 stderr:     self._sock = self._gevent_sock_class(family, type, proto, fileno)
2020/04/04 14:15:13 stderr: OSError: [Errno 97] Address family not supported by protocol
2020/04/04 14:15:13 Forked function has terminated: exit status 1

I got this from locustio/locust#556

@@ -5,6 +5,8 @@ COPY --from=watchdog /fwatchdog /usr/bin/fwatchdog
RUN chmod +x /usr/bin/fwatchdog

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Alternatively use ADD https:// (which will not be cached by Docker builder)

@@ -0,0 +1,56 @@
FROM openfaas/of-watchdog:0.7.2 as watchdog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM openfaas/of-watchdog:0.7.2 as watchdog
FROM openfaas/of-watchdog:0.7.6 as watchdog

RUN chmod +x /usr/bin/fwatchdog

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Alternatively use ADD https:// (which will not be cached by Docker builder)

return ret

if __name__ == '__main__':
#app.run(host='0.0.0.0', port=5000, debug=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#app.run(host='0.0.0.0', port=5000, debug=False)

@@ -0,0 +1,49 @@
FROM openfaas/of-watchdog:0.7.2 as watchdog
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM openfaas/of-watchdog:0.7.2 as watchdog
FROM openfaas/of-watchdog:0.7.6 as watchdog

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RUN chmod +x /usr/bin/fwatchdog

ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Alternatively use ADD https:// (which will not be cached by Docker builder)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be removed. I made this comment earlier

def handle(event, context):
return {
"statusCode": 200,
"body": "Hello from OpenFaaS!"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have this echo the request body, like in the default flask template handler

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I'm matching the existing templates. This is what the default http header does, so I don't want to change that.

I don't want to change anything about these templates across the board. I just want to make the debian ones available and functional.

At least under this PR.

Update the listening address for the WSGI server to use 0.0.0.0
instead of ''. This should resolve errors about Address family
not supported by protocol (Python OSError Errno 97.

Signed-off-by: Jeff Lowrey <[email protected]>
@Jeff-Lowrey
Copy link
Author

Jeff-Lowrey commented Apr 4, 2020

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.

@Jeff-Lowrey Jeff-Lowrey requested a review from alexellis April 6, 2020 18:46
return ret

if __name__ == '__main__':

Copy link
Contributor

@viveksyngh viveksyngh Apr 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using gevent based WSGIServer, we should monkey patch standard libraries to make it cooperative with gevent.

from gevent import monkey
monkey.patch_all()

http://www.gevent.org/api/gevent.monkey.html

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... well, ok.

But that kind of represents a change to all of the templates, which is out of the scope of this PR>

That's probably better as a new issue, especially because I at least don't know any implications for using a newer version of gevent..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what "we should money patch" means? 🤔 I would say this is also out of scope since the task was to port from Alpine to Debian. Happy for you to raise an issue Vivek if you can explain in a bit more detail there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gevent is built on top of libevent/libev (for asynchronous I/O) and greenlets (lightweight cooperative multi-threading). Greenlets are based on Stackless Python. Greenlets has no implicit scheduling and we can control exactly when our code runs.

In case of I/O block, the code will automatically switch to other I/O or wait for acknowledgment, however the standard libraries implementation does not work that way.

Monkey patching is required to patch the standard library with compatible cooperative counterparts from gevent package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that, this is out of scope of this PR. I will raise a separate issue.

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

Successfully merging this pull request may close these issues.

5 participants