search

openfaas / store

Official Function and Template Store for OpenFaaS
MIT License
162 stars 56 forks source link

Add AsciiWeather to Function Store #53

Open roncrivera opened 5 years ago

roncrivera commented 5 years ago

Hello everyone,

Happy New Year!

I would like to propose asciiweather function to be added into the function store.

I think this would be a good sample/demo function for getting started with OpenFaas. The associated github repo can also serve as a reference for converting a Go CLI application into a serverless function.

Here are the function definitions for amd64 and armhf for the registry.

amd64:

{
    "title": "AsciiWeather",
    "description": "This is a ported version of the wego weather client as an OpenFaaS serverless function.",
    "image": "roncrivera/asciiweather:latest",
    "name": "asciiweather",
    "fprocess": "xargs ./wego",
    "repo_url": "https://github.com/roncrivera/openfaas-asciiweather"
}

armhf:

{
    "title": "AsciiWeather",
    "description": "This is a ported version of the wego weather client as an OpenFaaS serverless function.",
    "image": "roncrivera/asciiweather:armhf",
    "name": "asciiweather",
    "fprocess": "xargs ./wego",
    "repo_url": "https://github.com/roncrivera/openfaas-asciiweather"
}

Here it is in action:

Thank you very much for your time.

best regards, ron

ivanayov commented 5 years ago

Happy New Year @roncrivera !

Thank you for opening this. Is the function written/tested for arm only?

Best, Ivana

alexellis commented 5 years ago

There seems to be amd64 available too.

@ivanayov please can you evaluate the submission and give feedback?

@roncrivera if accepted I would like to move the repo into the faas-and-furious org so we can keep it up to date.

Alex

rgee0 commented 5 years ago

Hello @roncrivera

I really like the look of this. It took a little tracking down as the function definition detail above is different to the actual function definition. For ease for others who might be taking a look the stack.yml is:

provider:
  name: faas
  gateway: http://127.0.0.1:8080
functions:
  asciiweather:
    lang: dockerfile
    handler: ./asciiweather
    image: riverron/asciiweather:latest

A couple of questions:

What can be done about the spacing across the wind speed lines? It looks like the occurrence of a diagonal arrow might be a factor
image

What are your thoughts on how this displays in the UI? I understand that this is largely a terminal application but since accessing the Store via the UI is one of the first contacts for new users I think it important that the output makes immediate sense.

rgee0 commented 5 years ago

BTW, did you build your image with your API key built in? Your image worked for me without any changes.

Is there a way this can be extracted into secrets / env vars?

roncrivera commented 5 years ago

Hello @rgee0, thanks for the feedback.

To answer your questions:

  1. Let me try tracking the cause of the misalignment of the wind speed lines. The main source is in Go which I am still in the process of learning so this might take a while; the good thing about it is I now have a reason not to procrastinate. ;-)
  2. I was mainly testing in the CLI and didn't realise it looked differently in the UI. I wonder if it's just a matter of wrapping the http response inside a <pre></pre> tag so they get rendered correctly. Any idea if this is possible?
  3. I've created a shell wrapper that will fetch the API key from the secret object to make it more secure. I've pushed the updated images accordingly. Thanks for catching this.

@alexellis Happy to move the repo to the appropriate location if this gets accepted (hopefully).

roncrivera commented 5 years ago

Hi @rgee0,

For point 2 above, it seems like the ANSI escape codes for printing the output in color is not being rendered correctly in the browser.

I’ve tested an option (-aat-monochrome=true) to print the output in monochrome and it looks much better.

I am going to rebuild the image with this option as the default for a better experience in the UI.

I will also update the README with instructions to switch it off in the CLI.

alexellis commented 5 years ago

If this needs an API key our UI doesn't support adding that yet, so it would unfortunately break.

There is an open issue asking for help to improve the store experience in the UI.

We might have to pause this issue for now - or have you write a short blog about it instead which I can share via @openfaas on Twitter?

Alex

alexellis commented 5 years ago

Derek add label: blocked

roncrivera commented 5 years ago

@alexellis No worries, will put this in the back-burner for now.

A blog about this is a good idea, I'll work on it.

Thanks everyone for all your time reviewing this.

alexellis commented 5 years ago

@roncrivera are you proxying the public SaaS service or running your own?