Skip to content

(WIP) Add option to package individually without moving module content to root folder (i.e. like base serverless) #279

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

Closed

Conversation

bweigel
Copy link
Contributor

@bweigel bweigel commented Nov 12, 2018

What it does

Adds option IndividuallyMoveUpModules to allow customization of packaging, when individually: true.

Let's see an example that tries to show, why this might be important...

We have a root-directory containing one lambda (as a module) located at ./fn, some tests (./tests) and our serverless.yml, giving this structure:

$ ls -lR
.:
total 80
drwxrwxr-x 2 bweigel bweigel  4096 Nov 12 18:14 fn
-rw-rw-r-- 1 bweigel bweigel   431 Nov 12 18:12 serverless.yml
drwxrwxr-x 2 bweigel bweigel  4096 Nov 12 18:15 tests

./fn:
total 52
-rw-rw-r-- 1 bweigel bweigel   0 Nov 12 14:08 __init__.py
-rw-rw-r-- 1 bweigel bweigel 156 Nov 12 16:49 main.py
-rw-rw-r-- 1 bweigel bweigel  61 Nov 12 16:49 other_module.py
-rw-rw-r-- 1 bweigel bweigel   0 Nov 12 14:15 requirements.txt
-rw-rw-r-- 1 bweigel bweigel  24 Nov 12 16:46 yam.py

./tests:
total 20
-rw-rw-r-- 1 bweigel bweigel  0 Nov 12 16:47 __init__.py
-rw-rw-r-- 1 bweigel bweigel 80 Nov 12 16:48 test_other_module.py

This is the content of the serverless.yml, where we define our function with its entry-point fn.main.lambda_handler located at the fn/main.py. We also define that we want individual packaging of functions using individually: true:

service: packaging_test

provider:
  name: aws
  runtime: python3.6

package:
  individually: true
  exclude:
    - '**/*'

functions:
  function-one:
    handler: fn.main.lambda_handler
    package:
      include:
        - 'fn/**'

Using base serverless functionality lets us package our (single function) service and we see, that the structure that is present in our root-folder has not changed. The module fn is still there:

$ sls package
Serverless: Packaging service...
Serverless: Excluding development dependencies...

$ zipinfo .serverless/function-one.zip
Archive:  .serverless/function-one.zip
Zip file size: 834 bytes, number of entries: 5
-rw-rw-r--  4.5 unx        0 bl stor 80-Jan-01 00:00 fn/__init__.py
-rw-rw-r--  4.5 unx      156 bl defN 80-Jan-01 00:00 fn/main.py
-rw-rw-r--  4.5 unx       61 bl defN 80-Jan-01 00:00 fn/other_module.py
-rw-rw-r--  4.5 unx        0 bl stor 80-Jan-01 00:00 fn/requirements.txt
-rw-rw-r--  4.5 unx       24 bl defN 80-Jan-01 00:00 fn/yam.py
5 files, 241 bytes uncompressed, 212 bytes compressed:  12.0%

However, if we were to use the serverless-python-packaging-plugin in our serverless.yml this would change:

# serverless.yml
...
plugins:
  - serverless-python-requirements
...
$ sls package
Serverless: Generated requirements from /home/bweigel/Projects/OSS/sls_package/fn/requirements.txt in /home/bweigel/Projects/OSS/sls_package/.serverless/requirements.txt...
Serverless: Copying from /home/bweigel/Projects/OSS/sls_package/.serverless/requirements.txt into /home/bweigel/Projects/OSS/sls_package/.serverless/fn/requirements.txt ...
Serverless: Installing requirements from /home/bweigel/Projects/OSS/sls_package/.serverless/requirements/requirements.txt ...
Serverless: Packaging service...
Serverless: Excluding development dependencies...
Serverless: Injecting required Python packages to package...

$ zipinfo .serverless/fn-packaging_test-dev-function-one.zip
Archive:  .serverless/fn-packaging_test-dev-function-one.zip
Zip file size: 727 bytes, number of entries: 5
-rw-rw-r--  3.0 unx        0 b- stor 98-Jan-01 00:00 __init__.py
-rw-rw-r--  3.0 unx        1 b- defN 98-Jan-01 00:00 requirements.txt
-rw-rw-r--  3.0 unx      156 b- defN 98-Jan-01 00:00 main.py
-rw-rw-r--  3.0 unx       61 b- defN 98-Jan-01 00:00 other_module.py
-rw-rw-r--  3.0 unx       24 b- defN 98-Jan-01 00:00 yam.py
5 files, 242 bytes uncompressed, 215 bytes compressed:  11.2%

Now the content of fn has moved to the root-level of the deployment artifact.
This is bad for a couple of reasons:

It breaks imports: fn/other_module.py imports a function from fn/yam.py using from fn.yam import .... However, this import statement will not work, when the content of fn is moved to the root of the zip-artifact, because now module fn will not be on our PYTHONPATH. It will cease to exists in our lambda runtime environment. I guess one could argue to just import like fn is the base folder and do from yam import ..., however this is where you break something else...

You break testing: tests/test_other_module.py contains unittests for some functions in fn/other_module.py. Again, inside test_other_module.py we use absolute imports from fn.other_module import .... This will work fine in the IDE (given it is configured correctly) until the Python interpreter parses the first line in other_module.py, which reads from yam import ... and the module yam cannot be found, because it is not on the path (only fn is...).

I guess you see the point that I am trying to argue. However you try to do it you will break something, either your lambda runtime or your tests. Something I would like to try and avoid and hence came up with this PR.

Implementation details

  • Don't break existing code bases. Keep default behavior as is.
    • if IndividuallyMoveUpModules: true (default behavior) moves module contents to root of deployment artifact (./fn/handler.py./handler.py)
    • if IndividuallyMoveUpModules: false leaves module contents as is (default serverless behavior; ./fn/handler.py./fn/handler.py)
  • During the writing of the new (individual) zip-file the option is checked in the function moveModuleUp
  • this will be configurable once accepted:
Variable Name Value Description
IndividuallyMoveUpModules false/true Default: true. Disable or enable moving module contents to the top of the zip-file artifact

TODO

  • implement feature
  • add simple test
  • fix tests
  • add more tests?
  • update documentation
  • iterate through feedback
  • await merge

related Issues

Update 2018-11-15

While working on this (i.e. on fixing failed tests) I also ran into some bugs, that have been lurking behind #209 and #194, which I will fix in this PR.

@dschep
Copy link
Contributor

dschep commented Nov 12, 2018

neat. I think this matches how I'd envisioned this working, but I wasn't using the feature, nor had time to develop my vision of how it should work.

@bweigel
Copy link
Contributor Author

bweigel commented Nov 13, 2018

Note to self: check funky behavior of requirement injection. Failed tests turn green, when hello4 is moved to first place in functions in serverless.yml ...
→ inconsistent requirement injection depending on order of defined functions (this might also be an issue in the plugin as is and is not covered by tests)

More context

Working → module defined in first function:

vlcsnap-2018-11-14-16h40m38s367

Failing → module defined in last function:

vlcsnap-2018-11-14-16h41m23s566

This might also cause the problems described in #209

@bweigel bweigel changed the title Add option to package individually without moving module content to root folder (like base serverless) (WIP) Add option to package individually without moving module content to root folder (i.e. like base serverless) Nov 15, 2018
@bweigel bweigel force-pushed the package_individually_like_base_sls branch from a83f907 to c370b75 Compare November 24, 2018 14:55
dschep pushed a commit that referenced this pull request Dec 3, 2018
hi @dschep 

I decided to split up and simplify PR #279 a little bit.
Started with porting some tests, which also helps towards resolutions of #269 ...
@bweigel bweigel force-pushed the package_individually_like_base_sls branch from c370b75 to 927b7cb Compare January 4, 2019 17:10
@bilby91
Copy link

bilby91 commented Jan 28, 2019

Looking forward on getting this PR merged, I've been using it and it's looking good

@bilby91
Copy link

bilby91 commented Jan 28, 2019

@bweigel One thing that I noticed while using this new configuration is that all of the directories that are siblings of the lambda function folder are copied as well.

For example, in this case, func1 zip contains func1 and func2 code, func2 zip contains func1 and func2:

func1/handler.py
func1/requirements.txt
func2/handler.py
func2/requirements.txt
serverless.yml
service: my-service

plugins:
  - serverless-python-requirements

provider:
  name: aws
  runtime: python3.7

package:
  individually: true

custom:
  pythonRequirements:
    slim: true
    IndividuallyMoveUpModules: false
    useDownloadCache: true
    useStaticCache: true

functions:
  func1:
    handler: func1.handler.lambda_handler
    module: func1
  func2:
    handler: func2.handler.lambda_handler
    module: func2

@bweigel
Copy link
Contributor Author

bweigel commented Jan 28, 2019

Hey @bilby91. Thanks for the comments. The feature is actually pretty much done. It's just dangling here, because some windows tests fail (if CI is to be believed :) ) and I have no idea why. I have not yet gone through the process of setting up a windows vbox and testing some on my own. So that's the gist of it.

One thing that I noticed while using this new configuration is that all of the directories that are siblings of the lambda function folder are copied as well.

This is wanted behavior (just like the vanilla serverless). If you want to include/exclude something from the actual source code you have to work with package.include|exclude.

@bilby91
Copy link

bilby91 commented Jan 28, 2019

@bweigel Awesome! Thanks for the feedback, make sense to solve the issue with include/exclude.

@brunoban
Copy link

Thank you so much for this!

@vinny1575
Copy link

Would love to see this merged. :) Thanks for the feature!

@frediana
Copy link

Hello, I can't wait for this PR to be merged too since I also have trouble invoke my functions when packaged individualy.

@bweigel
Copy link
Contributor Author

bweigel commented Aug 14, 2019

Anyone is invited to pick up the work. I am swamped right now and not in the mood to address broken windows tests....well this and merge hell :)

@vinny1575
Copy link

Could an admin look into re-invoking the tests? It looks like it's failing while setting up the environment... Could've been a fluke?

@bweigel
Copy link
Contributor Author

bweigel commented Jan 30, 2020

Closing to clean up my PRs. Will not work on this anymore.
Someone else can pick up here again if they want.

@bweigel bweigel closed this Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants