[NodeJS] Introduce NodeJS codegen for Google Cloud Functions #4406

jmuk · 2016-12-15T23:29:34Z

PR checklist

Read the contribution guildelines.
Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

This PR introduces a new language "nodejs-google-cloud-functions." This is quite similar to nodejs server but the index.js is formed to work within Google Cloud Functions.

wing328 · 2016-12-16T10:00:01Z

...er-codegen/src/main/java/io/swagger/codegen/languages/NodeJSGoogleCloudFunctionsCodegen.java

+        writeOptional(outputFolder, new SupportingFile("index.mustache", "", "index.js"));
+        writeOptional(outputFolder, new SupportingFile("package.mustache", "", "package.json"));
+        writeOptional(outputFolder, new SupportingFile("README.mustache", "", "README.md"));
+        if (System.getProperty("noservice") == null) {


@jmuk thanks for the PR. Instead of using noservice system property, what about using java -Dmodels -DsupportingFiles instead? https://github.com/swagger-api/swagger-codegen#selective-generation

You may also consider using .swagger-codegen-ignore to skip certain files when regenerating the code: https://github.com/swagger-api/swagger-codegen#ignore-file-format

re: .swagger-codegen-ignore: removed

re: noservice: well, this is what currently nodejs-server is doing. I think it's better to do things like that in another PR.

wing328 · 2016-12-16T10:01:11Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractNodeJSCodegen.java

+import java.util.Map;
+import java.util.Map.Entry;
+
+public abstract class AbstractNodeJSCodegen extends DefaultCodegen implements CodegenConfig {


@jmuk thanks for refactoring NodeJS generator into a common abstract class 👍

wing328 · 2016-12-16T11:38:22Z

@jmuk thanks for the contribution.

This PR introduces a new language "nodejs-google-cloud-functions." This is quite similar to nodejs server but the index.js is formed to work within Google Cloud Functions.

If only index.js is different, what about adding a CLI option to use different index.js mustache template instead of introducing another generator.

jmuk · 2016-12-16T21:31:35Z

To be more precise, not only index.js but also package.json (because it doesn't have to depend on some) and README.md are slightly different from nodejs-server -- but yeah, the contents are mostly common. Let me check how I can converge them into a single generator.

wing328 · 2016-12-17T02:56:12Z

@jmuk One way is to pass --additional-properties=enableGoogleCloudFunction=true via command line argument and then you can use the following in the mustache template

{{#enableGoogleCloudFunction}}
... code for enabling Google cloud function here ...
{{/enableGoogleCloudFunction}}

This does not add a new language, but adding some client options to support Google Cloud Functions (GCF).

jmuk · 2016-12-20T00:09:58Z

Updated. Now the single NodeJSServerCodegen.java does the codegen for GCF upon googleCloudFunctions=true additional property.

wing328 · 2016-12-20T13:40:31Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NodeJSServerCodegen.java

+            writeOptional(outputFolder, new SupportingFile("index.mustache", "", "index.js"));
+        }
+        writeOptional(outputFolder, new SupportingFile("package.mustache", "", "package.json"));
+        writeOptional(outputFolder, new SupportingFile("README.mustache", "", "README.md"));


Instead of using noservice, what about using selective generation as mentioned in https://github.com/swagger-api/swagger-codegen#selective-generation? Would that meet your need?

I prefer not to introduce another system property if the same output can already be achieved.

Actually this comes from the original NodeJSCodegen.java, I just moved the code location. I'm not adding new.

Also, the noservice is for a different purpose from the selective generation.

The nodejs-server codegen generates two files per API. For the example of petstore, it generates files like controllers/Pet.js and controllers/PetService.js (see https://github.com/swagger-api/swagger-codegen/tree/master/samples/server/petstore/nodejs/controllers).

When this noservice is specified, it omits the generation of PetService.js file or any other -Service.js files. Only Pet.js or those files.

@jmuk thanks for the clarification. I'll review and let you know if I've further question.

wing328 · 2016-12-20T13:41:51Z

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NodeJSServerCodegen.java

-        }
+        cliOptions.add(CliOption.newBoolean(GOOGLE_CLOUD_FUNCTIONS,
+            "When specified, it will generate the code which runs within Google Cloud Functions "
+                + "instead of standalone Node.JS server."));


SUGGESTION: A URL to tutorial on how to deploy the auto-generated code to Google Cloud Functions would be helpful for developers who are new to Google Cloud Functions.

Great idea. Done. Also added a section in README.mustache template, so people can refer to the URL after the code is generated.

Adds the client options help message and the README.md file.

wing328 · 2016-12-22T15:12:06Z

@jmuk PR merged into master. Thanks for your contribution.

wing328 · 2016-12-22T18:05:49Z

FYI. I merged #4411 into master to improve the NodeJS code quality a bit.

wing328 · 2017-02-15T14:06:45Z

modules/swagger-codegen/src/main/resources/nodejs-google-cloud-functions/index.mustache

@@ -0,0 +1,44 @@
+'use strict';


@jmuk we'll move this file to nodejs/google-cloud-functions/index.mustache (and update the code accordingly). so that all nodejs (server) related files are under the folder.

Are you OK with the proposed change?

Sure, I don't care the exact file location as long as the code works. But thank you for telling me!

I found that this file is the same as index-gcf.mustache so I simply removed it via #4808. I did some tests and the generator is still working properly with or without the option for Google cloud function.

* Another approach: extending NodeJS server to support GCF. This does not add a new language, but adding some client options to support Google Cloud Functions (GCF). * Add URLs for how to deploy the generated code. Adds the client options help message and the README.md file.

wing328 reviewed Dec 16, 2016

View reviewed changes

wing328 added the Server: Nodejs label Dec 16, 2016

wing328 added this to the v2.2.2 milestone Dec 16, 2016

wing328 added the Enhancement: Feature label Dec 16, 2016

wing328 mentioned this pull request Dec 18, 2016

[NodeJS] better code format for Node.js server #4411

Merged

3 tasks

Another approach: extending NodeJS server to support GCF.

6cc24af

This does not add a new language, but adding some client options to support Google Cloud Functions (GCF).

jmuk force-pushed the gcf branch from cc7dcc6 to 6cc24af Compare December 20, 2016 00:08

wing328 reviewed Dec 20, 2016

View reviewed changes

Add URLs for how to deploy the generated code.

25141ca

Adds the client options help message and the README.md file.

wing328 merged commit 27f1b6e into swagger-api:master Dec 22, 2016

wing328 reviewed Feb 15, 2017

View reviewed changes

wing328 changed the title ~~Introduce NodeJS codegen for Google Cloud Functions.~~ [NodeJS] Introduce NodeJS codegen for Google Cloud Functions Feb 21, 2017

[NodeJS] Introduce NodeJS codegen for Google Cloud Functions #4406

[NodeJS] Introduce NodeJS codegen for Google Cloud Functions #4406

Uh oh!

Conversation

jmuk commented Dec 15, 2016

PR checklist

Description of the PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Dec 16, 2016

Uh oh!

jmuk commented Dec 16, 2016

Uh oh!

wing328 commented Dec 17, 2016

Uh oh!

jmuk commented Dec 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wing328 commented Dec 22, 2016

Uh oh!

wing328 commented Dec 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants