Support skipping webpack on a per function basis #256

johnf · 2017-10-19T04:53:11Z

What did you implement:

Closes #255

How did you implement it:

We add webpack: false to functions where we want to skip webpack.
In index.js in the relevant hooks we check if this is set and just return. This will run the original hook.

How can we verify it:

Examples:
In serverless.yml add a function similar to

functions:
  get_oai:
    runtime: python2.7
    handler: src/oai.handler
    webpack: false

run sls deploy -f get_oai and the .serverless/get_oai.zip should contain everything in the project. Webpack will not run

Todos:

Write tests
Write documentation
Fix linting errors
Make sure code coverage hasn't dropped
Provide verification config / commands / resources
Enable "Allow edits from maintainers" for this PR
Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

HyperBrain · 2017-10-19T08:24:15Z

@johnf Thanks for the PR 👍 .

One question: Normally all functions with runtimes other than node should not be webpacked in a mixed project. So wouldn't it be better to disable webpack for all these runtimes rather than having the user to add a flag where the plugin actually knows that it would not work?
I.e. checking the runtime of the functions.
Having this duplicated and force the user manually to add the flags to get the project compile seems a bit cumbersome to me.

johnf · 2017-10-19T08:34:36Z

@HyperBrain That is an excellent point. I was so focussed on making my problem go away I didn't think of that!
I'll update with that change this weekend.

HyperBrain · 2017-10-19T08:26:13Z

index.js

-        .then(this.packageModules),
+      'before:deploy:function:packageFunction': () => {
+        if (this.options.functionObj.webpack === false) {
+          return;


All hooks must return promises. We cannot guarantee that the caller is aware that a non-promise can be returned that cannot be awaited. As we are a plugin we must make no assumptions about our host.

return BbPromise.resolve();

HyperBrain · 2017-10-19T08:28:48Z

index.js

-        .then(this.compile)
-        .then(this.wpwatch),
+      'before:offline:start': () => {
+        if (this.options.functionObj.webpack === false) {


Offline is used to simulate the service, not a single function (and will recompile in case of a file changed).
So imo the checks on offline:start and offline:start:init must be removed, as this would be never called.

HyperBrain · 2017-10-19T08:36:49Z

lib/validate.js

@@ -83,6 +83,10 @@ module.exports = {
      _.merge(entries, entry);
    } else {
      _.forEach(functions, (func, index) => {
+        const data = this.serverless.service.getFunction(func);
+        if (data.webpack === false) {


Check for the runtime here - not the manually set flag, as discussed in my comment.
Additionally you create an entries array here that is shorter than the actual number of functions in the service.
This might fail in the compile and deploy steps with individual packaging when the entries have to be mapped back to the actual functions by index. A better way would be to only iterate over the supported functions (i.e. node.js) in all places then, instead of doing the check within the for loop. That would catch all possible combinations.

const supportedFunctions = _.filter(functions, func => !func.runtime || _.includes([ 'nodejs6.10', ... ] func.runtime); _.forEach(supportedFunctions, (func, index) => {

HyperBrain · 2017-10-19T08:38:22Z

@johnf We also should check that the changes do not affect other providers like Google and OpenWhisk that currently work with the plugin. Not sure yet, how to do that - maybe we have to to some tests with the PR there.

johnf · 2017-10-22T08:25:44Z

@HyperBrain

So If we want to be 100% correct and support all the providers then I've come up with

    this.nodeRuntime = () => {
      const provider = this.serverless.service.provider.name;
      const runtime = this.options.functionObj.runtime || this.serverless.service.provider.runtime;
      
      const jsOnlyProviders = [ 'azure', 'google', 'webtasks' ];
      if (jsOnlyProviders.include(provider)) {
        return true;
      }
      
      if (provider === 'openwhisk' && runtime === null) {
        return true;
      }

      return /^nodejs/.test(runtime);
    };

I think this might be a bit tedious and will be prone to errors if things change in the future.

I think instead we assume that the top level runtime is a javascript runtime and then we just check if a specific provider has been set on the function. (I could document this in the readme).

this.nodeRuntime = () => {
      const runtime = this.options.functionObj.runtime;
      
      return !runtime || /^nodejs/.test(runtime);
    };

Thoughts?

HyperBrain · 2017-11-18T15:06:15Z

lib/validate.js

-    } else {
-      _.forEach(functions, (func, index) => {
-        const entry = getEntryForFunction.call(this, functions[index], this.serverless.service.getFunction(func));
+      if (this.hasNodeRuntime(serverlessFunction)) {


@johnf Did you check if this works with serverless invoke local in case you invoke a non-node function?
The entries object will end up empty in this case (because the single function is skipped - which is ok), but what will webpack do in this case?

If that works, I'll give it a go and it will be part of 4.1.0

@HyperBrain

I've done a bunch of testing of different scenarios. Amended the code to skip running webpack in the right places.

One caveat though, in a mixed project you have to package individually. This is because if you just do an sls deploy the serverless-webpack code takes over and only packages up the js files. It will ignore the rest of the project.

I've added a note to the README. Feel free to edit or move it somewhere else in there.

johnf · 2017-11-18T22:05:34Z

except I've broken the tests because of the way I added in hasNodeRuntime. Will review that a bit later have to head out

johnf · 2017-11-18T22:08:11Z

@HyperBrain Any thoughts on preferred approach here.
I added some common helper functions into index.js which works at runtime because the constructor runs, but doesn't work in the unit tests.
I could either put the helpers into a helpers.js and include them in every file or maybe just pull them in, in the tests themselves?

HyperBrain · 2017-11-20T09:58:04Z

Imo, the helper functions should be put into a separate file that exports them (to keep the same semantics throughout the project). I have one question to the empty config check - I don't completely understand the reason for that. Wouldn't it mean in this case, that there are no node functions at all in the project, or it is misconfigured? Is having a project with the plugin enabled but only Python methods defined valid? Can you please shed come ligt onto that?

johnf · 2017-11-25T04:30:28Z

@HyperBrain
I've split out the helper functions into helpers.js

With regards to the empty check it depends on the call path. For things like packModules an earlier hook will have gone through validate but I don't think I was able to pull the runtime info anymore. So it was easier to check if a webpack config had been generated and skip on that basis. It usually happens when something is called with -f python_module.

HyperBrain · 2017-12-01T17:56:29Z

Looks good now. I'll do some tests with our projects and fix the unit tests to test for the newly introduced code branches, so that Coveralls is happy again.

mjmac · 2018-01-19T23:05:23Z

Hi, is there anything I can do to help get this integrated? Just ran into this issue today and was excited to find that there might already be a solution, but then heard the sad trombone when I saw that it's not landed yet...

HyperBrain · 2018-01-20T10:13:48Z

@mjmac Oh sorry, I lost this entirely from my sight. If someone could add the unit tests, so that code coverage stays up again, I'll merge it (just do a PR on top of this one).

OrKoN · 2018-01-20T22:12:57Z

I have tried to add test but found that it's not easy because of mutations and mocks. Is there another way to skip Webpack for certain functions, perhaps, in https://github.com/serverless-heaven/serverless-webpack/blob/master/index.js so that the lib does not need to be changed a lot?

OrKoN · 2018-01-21T15:21:14Z

I have tried to do something myself over here https://github.com/serverless-heaven/serverless-webpack/compare/master...OrKoN:skip-non-node-functions?expand=1 It seems to work but the non-node functions get packaged weirdly: it looks like every file from the project is added to the archive and include/exclude settings are ignored.

  test:
    runtime: go1.x
    handler: go/bin/main
    package:
      individually: true
      exclude:
        - ./**
      include:
        - go/bin/main

HyperBrain · 2018-03-14T16:37:43Z

@OrKoN Sorry for the huge delay. I think the issue with packaging everything is, that with the conditional package property on the functions, Serverless' own packaging is triggered when it inspects these functions.

This leads to the fact that Serverless starts its complete packaging for the service and just overwrites webpacks package output (and Serverless will just include everything).

I think we must be a bit more experimental and sophisticated here and tune the in-memory service definition in a way that we hide any triggering parts from serverless or just call the very specific Serverless function packaging from our own packager, instead of having it act as fallback (and thus destroying all our outputs)

OrKoN · 2018-03-17T09:03:11Z

@HyperBrain no problems. It seems like it's a very hacky workaround :-) For now, I decided that I am not going to mix different languages in one project. But the feature seems to be very useful, eventually.

liam-betsworth · 2018-07-02T14:03:28Z

@HyperBrain is there any progress on this?

I'm also coming up against this problem using a serverless configuration with multiple environments.

This is a real deal breaker

HyperBrain · 2018-07-04T17:52:04Z

Hi @liam-betsworth , I agree that this issue gets more pressing as more runtimes appear (at least on AWS). As this implementation is somehow hacky, I'm still thinking about a proper solution.
The main solution would be to mark and remember all non-JS functions while the plugin iterates the service functions. Then it should skip the modification of the function.package.artifact property for the found functions. That will lead to a Serverless packaging for these non-JS functions (with all its drawbacks). However this approach would only work with individual packaging, but I think that should be ok.

spoecker · 2018-09-12T12:56:24Z

Are there any updates on this? I am also trying to mix nodejs and python functions in one serverless project.

musicalmacdonald · 2018-12-20T23:25:55Z

Since this PR hasn't been merged yet; Do you have suggestions on forcing this behavior without changing the plugin? Is there a way to use webpack.config.js to skip over functions?

gdugernier · 2019-01-16T14:06:28Z

Since this PR hasn't been merged yet; Do you have suggestions on forcing this behavior without changing the plugin? Is there a way to use webpack.config.js to skip over functions?

Facing the same issue. I have both python2.7 and JS functions in my project. I'm currently I'm using lerna and have 2 packages: one with the JS functions and serverless-webpack, and one with the python without serverless-webpack. It's somewhat hacky and necessitate some cross-packages ${cf:}, but it's doing the trick.

I'm not sure it's the most elegant way but since I was already using Lerna to separate lambda resources and database/vpc resources it felt the easiest way to implement

gparonitti · 2019-06-24T10:43:39Z

Any update on this? What does it need to get integrated? This feature would be of much help 😄 .

jagregory · 2019-08-16T01:51:07Z

Any update on this?

I'm currently maintaining a forked version of serverless-webpack for this functionality. It's not ideal.

ineffyble · 2019-10-21T23:26:57Z

Any updates on this? Very desired functionality.

stlouisweb · 2020-02-04T20:02:47Z

@HyperBrain can someone review #530? It appears to be a competing for this functionality.

miguel-a-calles-mba

Hi, may you please address the merge conflicts? Thanks.

j0k3r · 2021-04-08T15:22:11Z

See #579 available since 5.3.3

nponeccop · 2021-04-08T16:34:30Z

I almost started working on resolving the conflicts yesterday lol.

@j0k3r you can use me as a human merge conflict resolution bot. Which PR should I rebase so you merge it and I work on next one?

HyperBrain requested changes Oct 19, 2017

View reviewed changes

johnf mentioned this pull request Nov 9, 2017

Skip webpack on a per function basis #255

Closed

HyperBrain reviewed Nov 18, 2017

View reviewed changes

Support skipping webpack on a per function basis

a5600df

HyperBrain mentioned this pull request Jan 31, 2018

Deploy node and Java lambdas within same project serverless/serverless#4698

Closed

gdugernier mentioned this pull request Jan 24, 2019

Need ability to disable specific functions by name #454

Open

ineffyble mentioned this pull request Oct 21, 2019

Add support for multiple runtimes in service #530

Closed

7 tasks

smil2k mentioned this pull request Apr 18, 2020

Check for node runtimes first #579

Merged

7 tasks

miguel-a-calles-mba reviewed May 13, 2020

View reviewed changes

j0k3r closed this Apr 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support skipping webpack on a per function basis #256

Support skipping webpack on a per function basis #256

johnf commented Oct 19, 2017 •

edited

Loading

HyperBrain commented Oct 19, 2017 •

edited

Loading

johnf commented Oct 19, 2017

HyperBrain Oct 19, 2017

HyperBrain Oct 19, 2017

HyperBrain Oct 19, 2017

HyperBrain commented Oct 19, 2017

johnf commented Oct 22, 2017

HyperBrain Nov 18, 2017 •

edited

Loading

johnf Nov 18, 2017

johnf commented Nov 18, 2017

johnf commented Nov 18, 2017

HyperBrain commented Nov 20, 2017

johnf commented Nov 25, 2017

HyperBrain commented Dec 1, 2017

mjmac commented Jan 19, 2018

HyperBrain commented Jan 20, 2018

OrKoN commented Jan 20, 2018

OrKoN commented Jan 21, 2018

HyperBrain commented Mar 14, 2018

OrKoN commented Mar 17, 2018

liam-betsworth commented Jul 2, 2018

HyperBrain commented Jul 4, 2018

spoecker commented Sep 12, 2018

musicalmacdonald commented Dec 20, 2018

gdugernier commented Jan 16, 2019

gparonitti commented Jun 24, 2019

jagregory commented Aug 16, 2019

ineffyble commented Oct 21, 2019

stlouisweb commented Feb 4, 2020

miguel-a-calles-mba left a comment

j0k3r commented Apr 8, 2021

nponeccop commented Apr 8, 2021 •

edited

Loading

Support skipping webpack on a per function basis #256

Support skipping webpack on a per function basis #256

Conversation

johnf commented Oct 19, 2017 • edited Loading

What did you implement:

How did you implement it:

How can we verify it:

Todos:

HyperBrain commented Oct 19, 2017 • edited Loading

johnf commented Oct 19, 2017

HyperBrain Oct 19, 2017

Choose a reason for hiding this comment

HyperBrain Oct 19, 2017

Choose a reason for hiding this comment

HyperBrain Oct 19, 2017

Choose a reason for hiding this comment

HyperBrain commented Oct 19, 2017

johnf commented Oct 22, 2017

HyperBrain Nov 18, 2017 • edited Loading

Choose a reason for hiding this comment

johnf Nov 18, 2017

Choose a reason for hiding this comment

johnf commented Nov 18, 2017

johnf commented Nov 18, 2017

HyperBrain commented Nov 20, 2017

johnf commented Nov 25, 2017

HyperBrain commented Dec 1, 2017

mjmac commented Jan 19, 2018

HyperBrain commented Jan 20, 2018

OrKoN commented Jan 20, 2018

OrKoN commented Jan 21, 2018

HyperBrain commented Mar 14, 2018

OrKoN commented Mar 17, 2018

liam-betsworth commented Jul 2, 2018

HyperBrain commented Jul 4, 2018

spoecker commented Sep 12, 2018

musicalmacdonald commented Dec 20, 2018

gdugernier commented Jan 16, 2019

gparonitti commented Jun 24, 2019

jagregory commented Aug 16, 2019

ineffyble commented Oct 21, 2019

stlouisweb commented Feb 4, 2020

miguel-a-calles-mba left a comment

Choose a reason for hiding this comment

j0k3r commented Apr 8, 2021

nponeccop commented Apr 8, 2021 • edited Loading

johnf commented Oct 19, 2017 •

edited

Loading

HyperBrain commented Oct 19, 2017 •

edited

Loading

HyperBrain Nov 18, 2017 •

edited

Loading

nponeccop commented Apr 8, 2021 •

edited

Loading