-
Notifications
You must be signed in to change notification settings - Fork 293
(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
Conversation
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. |
Note to self: check funky behavior of requirement injection. Failed tests turn green, when This might also cause the problems described in #209 |
a83f907
to
c370b75
Compare
c370b75
to
927b7cb
Compare
Looking forward on getting this PR merged, I've been using it and it's looking good |
@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:
|
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.
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 |
@bweigel Awesome! Thanks for the feedback, make sense to solve the issue with include/exclude. |
Thank you so much for this! |
Would love to see this merged. :) Thanks for the feature! |
Hello, I can't wait for this PR to be merged too since I also have trouble invoke my functions when packaged individualy. |
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 :) |
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? |
Closing to clean up my PRs. Will not work on this anymore. |
What it does
Adds option
IndividuallyMoveUpModules
to allow customization of packaging, whenindividually: 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 ourserverless.yml
, giving this structure:This is the content of the
serverless.yml
, where we define our function with its entry-pointfn.main.lambda_handler
located at thefn/main.py
. We also define that we want individual packaging of functions usingindividually: true
: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:However, if we were to use the
serverless-python-packaging
-plugin in ourserverless.yml
this would change:$ 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 fromfn/yam.py
usingfrom fn.yam import ...
. However, this import statement will not work, when the content offn
is moved to the root of the zip-artifact, because now modulefn
will not be on ourPYTHONPATH
. It will cease to exists in our lambda runtime environment. I guess one could argue to just import likefn
is the base folder and dofrom yam import ...
, however this is where you break something else...You break testing:
tests/test_other_module.py
contains unittests for some functions infn/other_module.py
. Again, insidetest_other_module.py
we use absolute importsfrom fn.other_module import ...
. This will work fine in the IDE (given it is configured correctly) until the Python interpreter parses the first line inother_module.py
, which readsfrom yam import ...
and the moduleyam
cannot be found, because it is not on the path (onlyfn
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
IndividuallyMoveUpModules: true
(default behavior) moves module contents to root of deployment artifact (./fn/handler.py
→./handler.py
)IndividuallyMoveUpModules: false
leaves module contents as is (default serverless behavior;./fn/handler.py
→./fn/handler.py
)moveModuleUp
IndividuallyMoveUpModules
false/true
true
. Disable or enable moving module contents to the top of the zip-file artifactTODO
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.