-
Notifications
You must be signed in to change notification settings - Fork 293
Skip package/deployment steps if nothing changed #148
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
So... this feature sounds like it needs to be a serverless feature itself, not a serverless-python-requirements feature. Given the scope of this plugin, however, I implemented a proof-of-concept that does something similar to this feature and might do enough of what you want. It doesn't re-gather the requirements if requirements.txt hasn't changed. Please see #162 . This helps us cut down on build times greatly, but it of course still re-generates the zip file. Just my 2c, but I'd say that the scope of this issue is too large, and should be "won't fix" because it needs to be implemented at the serverless level, not at the python requirements plugin level. |
Hi @AndrewFarley thanks for replying, but that's my point, it is already a Serveless feature. |
@Adriayala I honestly had no idea that serverless even did this until you mentioned it (I've always had to be using this plugin). I just did a brief test of that feature, and you're right... it says I just peeked at the serverless code that detects and does this feature. It basically still generates the .zip file locally, and then compares the hash of the local zip file against the hash of the file that was deployed. So, what that says to me, is the process of re-generating the pip dependencies doesn't have consistent output. I believe a future version of my proposed merge, re-using the same exact dependencies, will actually make this feature work right. I'm working out the kinks, but from what I can see, I believe it will. This really sounds like a wonderful feature, and I'd love to see it as well. I'm going to do some initial tests with some of my ideas about my PR (moving the data into a temp folder) to see if I can get this state to trigger at least once. If I can, then we're on the right path. Thanks for bringing this to light. Keep you posted... |
(to try it, install the v4 beta with |
@dschep You're on the right track. I was just digging into this and saw
|
Let me know if you have another beta to try, I've a few projects I can test them on. |
Will do @AndrewFarley |
While workin' on my merge... this problem piqued my interest. I noticed we're using zip-local module which is basically abandonware. I recommend modifying this plugin's zipping to use the same module that serverless does, which is a well supported and widely used module node-archiver. Anyone interested feel free to pick this up. Please see: Which is a reference from the serverless framework of how they do it. I believe that archiver module properly sets the modification/creation times based on your input of all folder paths for a file you add to a zip. |
I like the idea of using the same zip code. I initially did exactly that: https://github.com/UnitedIncome/serverless-python-requirements/blob/072915b546f53a914cae9c0b4ac01c0d6dffcd9b/lib/zipService.js |
Hey, I'm glad to see you guys are making progress in here. So what's the plan, using that zip code instead of the current one? and is there an ETA? |
It's not on my(@unitedincome's) roadmap, but I'll merge it as soon as it's done. You can follow @AndrewFarley's progress on #165 |
I would love to work on this next I know I could use it, but I’m neck deep with my other merge hoping to finish that in a week or two. But if I can I’d say maybe a month and I’ll have some code for this then will just need some reviewing. |
Hi, is someone still looking into this? I don't know if the addition of lambda layers changes how this is behaving. Running serverless 1.38.0 under macos I still have to upload both lambda and layers .zips every time we deploy a service, even if anything changes. |
Hi, is someone working on this? The issue is still up, and uploading even nothing has changed takes too much time. |
No one is working on this, but if anyone wants to pick this up, please do. |
At the risk of crossing requirements here, it would seem there's an opportunity here to add a "skip layer building" option as well...? The idea being that if this plugin has been used to deploy python requirements as an independent layer, it should also support options for the user to choose to independently build (and deploy) either the requirements layer or the function code? |
Really really would appreciate this, its a major pain point for us |
Where are we with this feature request? My functions are being rebuild each time I deploy even if they or the requirements haven't changed at all. |
I'm interested in working on this, but I'm a bit confused on the state of things, since both the items mentioned above as potentially resolving this issue are now done:
Did those perhaps just turn out not to have the impact on this issue that you thought? @AndrewFarley What are the next steps here? ("Figuring out the next steps" is a fine answer; just wanted to check before I start off in potentially the wrong direction 🙂) |
@brettdh There's some history from before when I joined this codebase, but the serverless framework does use zip-local and the solution to this should be moving this plugin over to that framework which would allow the timestamps to be set properly and then would be able to detect the lack-of-changes, as the serverless framework does. |
As far as static-caching, that MR allows caching of having to re-run pip to compile modules, but when it is injected into the serverless package because of timestamps of the folder as a result of a flaw of the jszip plugin it will never generate the same .zip file from every build. |
Are you sure? I don't see it in the package.json, but I do see jszip there. The earlier message said zip-local had the timestamp flaw; are you saying jszip has a similar flaw?
I'm confused; do you mean moving this plugin to use a different zip module? (I'm not sure what "moving this olugin over to that framework" means otherwise.) And which zip module will set the timestamps correctly? At first I thought it was node-archiver, but from your notes today I'm not sure. |
@brettdh Actually, you're right someone must have merged something without me noticing that we've switched over to jszip. So, then this issue should be resolved already, or it should be fairly trivial to resolve. Aka, there might be some slight tweaks necessary to our usage of jszip to ensure we do things "the same". Making our inputs generate exactly the same binary (zip) output, which serverless detects and will then skip. The problem previously when using zip-local was that all folders created had an date which was a timestamp of now and this was non-configurable. I haven't looked into this issue in a while now, but hopefully with this insight you can debug this. If you'd like to look into this please do. Just run (Note: IIRC, the serverless framework makes all folders/files it creates in the .zip file to have "epoch" as the date/time for consistency between builds) |
Yep, happy to look into it further, especially if what remains might be trivial to resolve. 🙂 I'm also curious about whether you'd expect the static caching and detection-of-no-changes also would apply to the .zip generated for a lambda layer (assuming we resolve whatever jszip config issue remains), or if further changes would be needed to address that. |
@brettdh The static caching should be unaffected by the desire for and/or changes as a result of this feature. If you need to tweak anything, it would be in the "copy" step into the .zip file (eg: to change its datetime to epoch). This code lives "outside" of the static caching logic. Note: This feature/concept has never been possible with this plugin. By fixing this, you'd probably make a ton of people that use this plugin really happy and giddy. |
Update: found the spot that needed a fixed timestamp; set the fixed timestamp; got a consistent .zip file output on each |
Hi @brettdh, were you able to pinpoint what's the issue with your zip files? If not, or you need a fresh pair of eyes to look into the issue, I'll be happy to assist. I'd really like to get this feature merged 😉 |
It seems #644 doesn't fully solve this. The compression step is skipped but a new layer still gets deployed even if the requirements didn’t change. |
I'm suffering with the exact same issue. After some investigating, I found the same thing with @relsunkaev
In some cases, different last modification time makes some troubles to cache. |
Would it be possible to completely skip the package and deploy steps if nothing actually changed? equal to what the vanilla Serveless does, if no code, dependencies and serverless.yml files changed it skips all the package, upload to provider and deployment steps.
At the moment, if you run
sls deploy
with this plugin, and re-run it again without any changes at all, it will re-package and re-deploy everything again (as a new version).This improvement would help a lot in cases like mine, where we have a CD/CI based on a Monorepo, that will automatically deploy and skip when there were no changes.
The text was updated successfully, but these errors were encountered: