-
Notifications
You must be signed in to change notification settings - Fork 293
Fatal error when trying to deploy single-functions #161
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
Comments
Encountered this issue today, also with SLS 1.26.1, plugin 4.0.0. |
Did a little more digging, it seems the plugin is trying to access the I do not know whether it is supposed to find the "whole project" zip-file (e.g. |
How did I miss this bug before I released 4.0.0 😢 Weird that the tests pass since they only have 1 function and no package artifact specified. It should be finding the "whole project" zip-file as you say @DHager, unless of course that package.individually is set. |
Hey @AndrewFarley & @DHager did I miss something? I think 43e1404 should fail tests, but it doesn't. |
@dschep While both seem to trigger the same "after" handler from plugin-hooks, it looks like when running I suspect what's happening is that a non-null packageAll() {
const zipFileName = `${this.serverless.service.service}.zip`;
return this.resolveFilePathsAll().then(filePaths =>
this.zipFiles(filePaths, zipFileName).then(filePath => {
// only set the default artifact for backward-compatibility
// when no explicit artifact is defined
if (!this.serverless.service.package.artifact) {
this.serverless.service.package.artifact = filePath;
this.serverless.service.artifact = filePath;
}
return filePath;
})
);
}, |
In other words, I think it boils down to "The Possible approaches:
|
If that is really the case, we should submit a patch upstream instead of fixing our plugin. However, I just tested, and it seems this problem is only in the latest (4.0.0) version of this plugin, if I force-use a previous release this problem goes away, which means, it's something we can fix and/or something that broke in the latest release. I admit, it is really annoying to not be able to do single-function deploys assisting in more rapid development, so I hope someone can fix this soon. I'll get around to it eventually if no one else does. |
I agree that an upstream patch is desirable @AndrewFarley. I asked over in the serverless-contrib slack if anyone had thoughts on it but got no responses 🤷♂️ |
Created an issue upstream ☝️ |
So, upstream patches aside, this works fine right now with pre-4.0 of this plugin. So, to me that says there is something we can do right now without having to wait for them to fix this upstream, no? |
Not without ditching the "inject deps directly into the zip" change made in 4.0. It worked before because this plugin wasn't touching the zip at all, it was making symlinks before serverless created the zip file. |
Ahh, didn't know that change happened in 4.0. Only started really diving in the code with 4.0 . :) . Thanks for clarifying |
Yup. No worries 👍 |
* Better mixed runtime & function deploy handling fixes #161 and fixes #179 * another tweak * fix again? * Fix corrupted zip archive in case of same module * Do not try to install requirements for non-python runtime * Fix lint * format * update test for merging #181 * @AndrewFarley's fix * huh. depcheck sucks. * fixix syntax error
The problem still exists on current plugin version, how to fix this? |
@alexandreesl This should be fixed in the latest version, as of 4.0.3 which came out before you commented. It seems to be fixed for me as well, and I was the original reporter of this bug. If you're still having this bug, can you share with us your serverless.yml and system information and any other details you think are relevant? Thanks! |
EDIT: #218 seems related I am still encountering this on 4.1.0. I am using dockerizePip and individual packaging. This is a node-python mixed project, with two Python modules and several other Node modules. Deploying the whole stack works fine.
npm ls
|
Fixes #157 (filed by me) ## What this does * Makes the download caching of pip a "first-class citizen" as an option directly in this plugin's options. This will "fix" a few (attempts) at using the pip cache, specifically in Docker, and will simplify this feature (as the user simply has to enable it, not specify a folder). In a future MR, I'd highly suggest enabling this by default. * Second, it adds a new type of caching called "static caching" which allows you to cache the outputs of this plugin. This greatly speeds up every single build as long as you have the feature enabled and do not change your requirements.txt file. In a future MR, I'd highly suggest enabling this by default also. * The pip download and static cache are shared between any projects of the same user through an [appdir](https://www.npmjs.com/package/appdirectory) cache folder when packaging your service. This _especially_ helps on projects that heavily use Docker (Win/Mac) for deployments or development, or for pip modules that need to compile every time, and _especially_ for projects with long requirements.txt files. This will also greatly help the longer and more complex your requirements.txt is, and/or if you use the same requirements.txt on multiple projects (common in team environments). ## Implementation details * When either cache is enabled, this plugin now caches those requirements (download or static) to an "appdir" cache folder (per the [appdirectory](https://www.npmjs.com/package/appdirectory) node module). * When this feature is NOT enabled, nothing changes * Injection happens directly from the new cached requirements directory via a symlink created in the right place in `.serverless` or `.serverless/functionname` if deploying individually. * As mentioned above, there is a symlink into the .serverless folder when the static cache is enabled pointing to it, so you still "know" where your cache is (for both individually and non-individually packaged functions). * The requirements.txt "generator" was improved to remove comments, empty lines, and sort the list of items before trying to use it (or check its md5 sum). This allows for more actual md5 matches between projects, in-case of comments and such in the requirements file. * A new command was added to the command-line to flush the download/static cache, called cleanCache invokable with: `serverless requirements cleanCache`. This clears all items including the download and static cache. * A handful of new tests were created for various edge conditions I've found while doing this refactoring, some were based on bugs other people found while using this plugin with some combination of options, some are not directly related to this merge's intent, but it's just part of my stream of work/consciousness. Sorry tests take a lot longer to run now since there are lots more now. * A UID bug fix related to docker + pip was implemented (seen on a few other bugs) from @cgrimal * The following new configurable custom options were added to this plugin... Variable Name | Value | Description --- | --- | --- useStaticCache | `false/true` | Default: false. This will enable or disable the static cache. After some testing I would like to make this default: true, as this will greatly help everyone, and there's no reason to not enable this. Possibly making this default: true will help weed out issues faster. I'll gladly step-up to quickly fix any bugs people have with it since I'm now well accustomed with the code. useDownloadCache | `false/true` | Default: false. This will enable or disable the pip download cache. This was previously the "example" code using a pipEnvExtraCmd to specify a local folder to cache downloads to. This does not require a cache location to be set, if not specified it will use an appdirs.usercache() location. cacheLocation | `<path>` | Default: [appdirectory](https://www.npmjs.com/package/appdirectory).userCache(appName: serverless-python-requirements) This will allow the user to specify where the caches (both static and download) should be stored. This will be useful for people who want to do advanced things like storing cache globally shared between users, or for CI/CD build servers on shared-storage to allow multiple build machines to leverage a cache to speed builds up. An example would be to mount a shared NFS store on all your CI/CD runners to `/mnt/shared` and set this value to `/mnt/shared/sls-py-cache`. staticCacheMaxVersions | `<integer>` | Default: 0. This will restrict the a maximum number of caches in the cache folder. Setting to 0 makes no maximum number of versions. This will be useful for build/CI/CD machines that have limited disk space and don't want to (potentially) infinitely cache hundreds/thousands of versions of items in cache. Although, I would be disturbed if a project had hundreds of changes to their requirements.txt file. ## TODO - [X] Feature Implementation - [X] BUG: Deploying single-functions fails (Packaging works, but fails because of #161 ) - [X] Code Styling / Linting - [X] Test to be sure Pipfile / generated requirements.txt still works - [X] Tested a bunch on Mac / Linux with and without Docker - [X] Adding Tests for Download Cache - [X] Make sure zip feature still works - [X] Ensure all existing tests pass - [X] Adding Tests for static cache - [X] Updating README.md to inform users how to use it - [X] Make sure dockerSsh works - [X] Implement error when trying to use --cache-dir with dockerizePip (won't work) - [X] Implement suggestion when trying to use --cache-dir without dockerizePip - [x] Test on Windows - [x] Iterate through any feedback - [x] Rebase with master constantly, awaiting merge... :) Replaces #162
FYI: This is with the latest code from master
Version: serverless@1.26.1
Steps to reproduce: Create any/simple stack which uses this plugin, and try to do a single-function deploy.
master/bleeding edge might not be ready yet, I was just developing something on top of it and ran into this.
Cheers!
The text was updated successfully, but these errors were encountered: