-
Notifications
You must be signed in to change notification settings - Fork 105
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
Could we move aws-sdk to devDeps? #144
Comments
...Almost 10MB with the current deps. Is there a reason that the full aws-sdk needs to be included? |
Hi @tabroughton, I honestly do not remember why I ended up having aws-sdk as a dependency, I recall there was some talk here in the issues; as you can see the project is quite old and I stopped using it in production several years ago, I am just trying to maintain it now. I can't say for sure if the vast majority of people using this are on AWS Lambda; anyway, when you say "though I'm curious why that would be needed", what do you mean? What's the intended way? I'm genuinely asking since I haven't had the chance to work with node since years, and now there might be more robust solutions. |
I mention about the aws-sdk because AWS Lambdas already have the aws-sdk installed so there's no need to have it listed as a dependency in for production in the package.json, if I do then I when I build my --production app to deploy to lambda I have a very large application to deploy. Too large for lambda@edge and also makes my normal lambdas very large. But if you take it out of the package.json and put it in the devDependencies instead then when I install it for production it wouldn't have the aws-sdk installed in the node_module. (the dependency is already on the lambda so that's fine). Only problem I can see are those folks using it in other environments such as EC2s they manage themselves and docker containters in ECS. When they package and deploy they might find they had the dependency missing, they would have to include aws-sdk alongside winston-cloudwatch as a dependency in whatever application they're deploying. It means they would have to change their code, your latest package wouldn't be backward compatible. - but I think that's acceptable if there were a mention on it on your readme. |
I see, first of all thank you for the detailed explanation. As of now I think it's like a short blanket, either way someone's feet are going to be cold :D I'll think about this and see if there are possibile solutions, even maybe two separated builds, I'm not sure, but I'll give this some thoughts. |
@lazywithclass I think the best solution is to replace the aws-sdk dependency with @aws-sdk/client-cloudwatch-logs. Version 3 of the AWS SDK for Javascript is modular and enables installation of independent service packages. This will greatly decrease the size of the dependency and the upgrade process is rather straightforward. |
+1 Encountered this on a project. AWS SAM duplicates your lambda NPM modules for every lambda. This is especially problematic for two reasons (1) it's a logging library, so it's very common that most/every lambda needs Edit: I also tried using |
I will have some time over the weekend and will try to add this to the module, as I understand the size has become a problem for a lot of you! I just need to understand how to preserve the previous behaviour, for example how to pass a proxy server to the config options. Sadly the docs do not come with an example or deep explanation. |
Much appreciated @lazywithclass! Right now we've gotten around the issue by being selective in what we copy into our SAM artifact with a negative glob:
Which works pretty well since it guards against this issue from all dependencies. In an earlier solution, that we didn't go with, I created a fork of this repo, moved aws-sdk to devDependencies, and got the result that I was looking for (reduced artifact). I realize there's probably more to the puzzle, like you mentioned the config options, but just a vote of confidence. |
I gave this a look, they changed pretty much everything, for example the way I would have to deal with tl;dr I am moving aws-sdk to devDependencies (as some of you suggested), add a notice about this in the README, then publish 3.0.0 version, with also an update of all dependencies. |
winston-cloudwatch@3.0.0 has aws-sdk into devDependencies. Please let me know if you have any troubles with that. |
i am seeing this error when runnning in docker - Error: Cannot find module 'aws-sdk'. could it be related to moving to dev-dependency ? is aws-sdk required to run this in production build? if so, why is it dev-dependency? so is the recommendation is to include aws-sdk separately ? i am assuming aws-sdk is used to send to cloudwatch which is the main purpose of this library? |
I'm not sure of all the use cases but I assume that this is largely to run on aws lambda and the aws-sdk is already there.
With it being a prod dependency then my --production packages that depend on winston-cloudwatch are too large for lambda@edge and I have to remove winston-cloudwatch and I've ended up just using console.log as a work around for now.
If we moved aws-sdk to devDependencies then I wouldn't have this issue and others who need to deploy the aws-sdk in their production package that uses winston-cloudwatch could define it in their own package (though I'm curious why that would be needed).
The text was updated successfully, but these errors were encountered: