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

Update the handler to match style of golang-http-template #16

Open
wants to merge 3 commits into
base: master
from

Conversation

4 participants
@kturcios
Copy link

kturcios commented Feb 21, 2019

Description

  • Update the template to match the (event, context) style of the golang-http-template.
  • Add dev build options
  • Update README with instructions on how to use the template
  • Add waitress as the production server

Motivation and Context

This will allow users to return custom responses for their functions.

  • I have raised an issue to propose this change (required)

Which issue(s) this PR fixes

Fixes #8
Fixes #15
Resolves #12

How Has This Been Tested?

  • All examples in README were built and tested for expected responses for python3-http
  • All examples in README were built and tested for expected responses for python3-http-armhf

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Signed-off-by: Kevin Turcios kevin_turcios@outlook.com

@derek derek bot added the new-contributor label Feb 21, 2019

Update the handler to match style of golang-http-template
- Created two new templates, python3-http & python3-http-armhf
- Added waitress as the production server for new templates

Signed-off-by: Kevin Turcios <kevin_turcios@outlook.com>

@kturcios kturcios force-pushed the kturcios:event-context-model branch from 46d89dd to cd97260 Feb 24, 2019

$ curl -i $OPENFAAS_URL/function/<fn-name>
```

## Event and Context Data

This comment has been minimized.

@pcorbel

pcorbel Feb 25, 2019

Is there an "official doc" where the contract with the watchdog is explained?
The goal is to know which field is mandatory, and which is not

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

Hi @pcorbel how is this question related to the Python template?

@@ -0,0 +1,48 @@
FROM armhf/python:3.6-alpine

This comment has been minimized.

@pcorbel

pcorbel Feb 25, 2019

The stock image 'library/python:3.6-alpine' is compatible with ARM.
So we can use

FROM python:3.6-alpine

and if the image is build on ARM, the ARM version will be pulled.
Tested on Raspberry pi OK 👍

This comment has been minimized.

@kturcios

kturcios Feb 25, 2019

Author

Oh I had no idea about that! You've tested it on the Raspberry Pi already?

This comment has been minimized.

@pcorbel

pcorbel Feb 26, 2019

Yes I've tested and it ran successfully

This comment has been minimized.

@kturcios

kturcios Mar 1, 2019

Author

I just tried this and it failed. How were you able to run the image on your Raspberry Pi? Did you have to make any other changes?

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

Tried what / what failed? Give steps please so we can try too.

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

This appears to work for me on armhf:

docker run -ti python:3.6-alpine /bin/sh

This comment has been minimized.

@kturcios

kturcios Mar 3, 2019

Author

I tried building the template with python:3.6-alpine and deploying it on the rpi, but the watchdog fails with the message Error reading stdout: EOF.

@@ -0,0 +1,68 @@
from flask import Flask, request, jsonify

This comment has been minimized.

@pcorbel

pcorbel Feb 25, 2019

No header?
I think it would be a good practice to put

#!/usr/bin/env python
# -*- coding: utf-8 -*-

everywhere

This comment has been minimized.

@kturcios

kturcios Feb 25, 2019

Author

I'm unfamiliar with this so I'll take a look into it

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

We haven't done this elsewhere so please propose it generically @pcorbel in a separate issue.

@@ -0,0 +1,48 @@
FROM python:3.7-alpine

This comment has been minimized.

@pcorbel

pcorbel Feb 25, 2019

Why base python3-http on 3.7 and python-http-armhf on 3.6?
I think it would be a good idea to sync the two of them

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Mar 2, 2019

Hi, is this still WIP?

Sync python versions for http templates and add headers
Signed-off-by: Kevin Turcios <kevin_turcios@outlook.com>

@kturcios kturcios changed the title [WIP] Update the handler to match style of golang-http-template Update the handler to match style of golang-http-template Mar 3, 2019

@kturcios

This comment has been minimized.

Copy link
Author

kturcios commented Mar 3, 2019

It should be ready now

README.md Outdated

Python OpenFaaS template with Flask
The Python Flask templates make use of the incubator project [of-watchdog](https://github.com/openfaas-incubator/of-watchdog) to handle higher throughput than the [classic watchdog](https://github.com/openfaas/faas/tree/master/watchdog) due to the process being kept warm.

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

I think this is technically correct but confusing / too-much-info for the people who may read it.

to handle higher throughput than the classic watchdog due to the process being kept warm.

README.md Outdated
faas template pull https://github.com/openfaas-incubator/python-flask-template
faas new --list
Languages available as templates:
Templates available under this repository:

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

under->in

README.md Outdated
- python27-flask
- python3-flask
```
- python3-flask-armhf
- python3-http*

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

python3-http* - why the suffix *?

README.md Outdated

Generate a function with one of the languages:
Notes:
- The templates listed with an asterik are the only ones that support additional control over the HTTP response and provide access to HTTP request details.

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

Not necessary to explain this, just show it in the examples.


# Wait a couple of seconds then:
For example, returning a dict object type will automatically attach the header `Content-Type: application/json` and returning a string type will automatically attach the `Content-Type: text/html, charset=utf-8` for you.

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

Should we mention flask given that this is not visible to the user? the template will?

README.md Outdated

echo -n content | faas invoke myfunction
## Example usage
### Custom Status Codes and Response Bodies

This comment has been minimized.

@alexellis

alexellis Mar 3, 2019

Member

Why is Bodies capitalisedbutExample usage` isn't?

Sentence case should be fine for both

@kturcios kturcios force-pushed the kturcios:event-context-model branch from 00f5099 to 3de7507 Mar 3, 2019

Update README
Signed-off-by: Kevin Turcios <kevin_turcios@outlook.com>

@kturcios kturcios force-pushed the kturcios:event-context-model branch from 3de7507 to 937ed2d Mar 9, 2019


ARG ADDITIONAL_PACKAGE
# Alternatively use ADD https:// (which will not be cached by Docker builder)
RUN apk --no-cache add curl ${ADDITIONAL_PACKAGE} \

This comment has been minimized.

@smoynes-tc

smoynes-tc Mar 14, 2019

As a user, I'd like to be able to install build dependencies and have them removed after pip installs dependencies.

Is that possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.