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

Node.js Stack Updates #1

Merged
merged 19 commits into from
Feb 12, 2020
Merged

Node.js Stack Updates #1

merged 19 commits into from
Feb 12, 2020

Conversation

lkingland
Copy link
Contributor

Minor tweaks to the Node.js Appsody FaaS stack. Synopsis:

  1. Updated the maintainer to be myself.
  2. Naming changes to favor "JavaScript" and "Function" where applicable.
  3. Example test in the default project template based on Tape.
  4. Health and Liveness endpoints updated to match faas-js-runtime's settings.

@lkingland
Copy link
Contributor Author

@lkingland Minor detail, but I recommend using an LTS version of Node.js. https://nodejs.org/en/about/releases/

Node 13 is bleeding edge but will never be an LTS version, and ultimately the only versions we will support are those that are LTS.

Thank you very much for pointing this out. I'll be sure to use LTS versions from now on.

@lkingland
Copy link
Contributor Author

@lkingland I'm curious if prettify changed these to" instead of '. It's a minor nit, but all of the Node.js projects I've worked on at Red Hat have had single quotes as the default style for strings.

Sure did. I've now updated the default quotations setting of Prettier to singles.

@lkingland
Copy link
Contributor Author

@lkingland I think it would be good to eventually expose the Fastify logger to user functions. The logger is how any STDOUT/ERROR is printed in the faas-js-runtime framework itself, and is preferred because the logs are structured JSON and that is the format that the OpenShift log scraper (which scrapes all Pod logs) expects. I wonder if attaching the log to the context instance would be a good route.

Definitely sounds like a good idea. That addition is now in nodeshift/faas-js-runtime#28 , and this PR now expects the logger to be available on the context.

@lkingland
Copy link
Contributor Author

lkingland commented Feb 11, 2020

I can't really decide how I feel about this. The faas-js-runtime framework is written so that an incoming HTTP request will/can be processed regardless of whether or not the incoming request is a cloud event. Azure, for example, will support an "HTTP Trigger" as well as events. Maybe this is something we should flag for consideration down the line.

I see now that the handler can be invoked by either a standard HTTP request or via a CloudEvent which makes that precondition check incorrect. It has been removed. I would ideally like to have one precondition check provided by default in the template for illustrative purposes, but nothing useful comes to mind.

image/Dockerfile-stack Show resolved Hide resolved
image/Dockerfile-stack Show resolved Hide resolved
stack.yaml Show resolved Hide resolved
templates/default/test/test.js Show resolved Hide resolved
@lance lance merged commit 9945fc7 into boson-attic:master Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants