Skip to content
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

Claudia requires aws-sdk dependency, AWS does not #82

Closed
kyeotic opened this issue Nov 8, 2016 · 34 comments
Closed

Claudia requires aws-sdk dependency, AWS does not #82

kyeotic opened this issue Nov 8, 2016 · 34 comments

Comments

@kyeotic
Copy link

@kyeotic kyeotic commented Nov 8, 2016

  • Expected behaviour:

Claudia should create or update a function that uses aws-sdk without it being installed in node_modules, especially when using the --use-local-dependencies flag.

Lambda provides this dependency as part of the environment, so not installing it saves lambda disk space, and this practice is encouraged by Amazon. Clauda should not require that it is installed.

  • What actually happens:
Error: Cannot find module 'aws-sdk'
    ...
cannot require ./lambda after clean installation. Check your dependencies.
  • Steps to reproduce the problem:

Use aws-sdk as a devDependency, and try to update.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

Claudia uses its dependencies locally, and you should not include Claudia as a production dependency, only as a development one. Claudi Bot builder does not use aws-sdk, so it does not need it.

If your code requires aws-sdk, and you do not want to deploy it, use it as an optional dependency (npm install aws-sdk -O) and then deploy using --no-optional-dependencies, this will exclude AWS SDK from the final package.

--use-local-dependencies just copies whatever you have locally. It's your job then to make sure development dependencies are not installed.

@gojko gojko closed this Nov 8, 2016
@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

--use-local-dependencies just copies whatever you have locally. It's your job then to make sure development dependencies are not installed.

Yea, that's my problem. Dev dependencies are not installed, and claudia throws an error. It shouldn't do that.

@gojko gojko added invalid and removed invalid labels Nov 8, 2016
@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

Claudia is trying to load your lambda.js module, so something there is requiring aws-sdk. Try loading it from the node console to see exactly what.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

My code is loading the aws-sdk, and it isn't there because it doesn't need to be there. It will run once it is in AWS Lambda, because that specific dependency is provided as part of the environment. You are not supposed to install it with the lambda, it is a waste of disk space.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

OK. this is expected behaviour (--local-dependencies are not intended to be used with --no-optional-dependencies, this is to allow people to manually package binaries etc). AWS SDK is not removed by default so users can control the version of the AWS SDK deployed with the package.

Once I fix the .npmrc problem you reported, I assume you'll just be able to use --no-optional-dependencies and exclude aws-sdk and imagemagick if you want.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

I don't think this should be expected behavior. Amazon has made specific exceptions for this rather large dependency so that developers can reduce disk space usage. Claudia should make a similar exemption without the need for conflicting CLI flags.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

it does make the exception, you can exclude it with --no-optional-dependencies. but claudia needs to load your lambda module to validate the package, without it it can't work. force-removing aws-sdk is too complicated, because of transitive dependencies.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

So in order to validate the package it must ensure that something that doesn't need to be there is there? I don't think that's a very developer-friendly solution. It might be difficult to get around, but I am sure there is a way. You could provide a stub or Claudia's version of the sdk. You should be validating that the package will work in lambda (it will), not that it works locally.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

as I mentioned in the other comment, you can achieve what you want by using aws-sdk as an optional dependency (npm install aws-sdk -O) and deploy with --no-optional-dependencies), but not mix it with --use-local-dependencies. I assume the problem with .npmrc is blocking you at the moment, but I'll fix that soon (and you can work around it by using your user-home-dir .npmrc)

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

That's the issue though, I want to --use-local-dependencies because its more efficient and deterministic than letting Claudia re-install everything. It's also required under some cases, like using pre-installed binaries.

You are telling me that if I want to --use-local-dependencies, that claudia does not and will not allow me to work without an optional dependency that Amazon encourages everyone to ignore.

You are saying that the best practices used by Amazon are exclusive to the best practices used by Claudia. That's a big deterrent to using Claudia.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

If you aren't going to fix this, will you provide a flag to skip validation? I don't see another way to use this tool.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

if you are using the claudia api builder or bot builder, your lambda function needs to be loaded so claudia can get the configuration. skipping validation won't make a difference in that case. we can skip validation for event-driven services.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

I don't understand your comment. What configuration are you getting by loading the lambda locally? Shouldn't the claudia.json contain all the info needed to deploy?

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

if you're using claudia api builder or bot builder, the api definitions are in the code, so claudia needs to load your lambda module to read the configuration. if you're not using api builder or bot builder, this is not needed and we can skip validation with a flag

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

Oh, gotcha. Well, looks like I can't use this then. Thanks for the explanation.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Nov 8, 2016

if you lazy-load aws-sdk (eg inside a function, not in the requires at the start of a file), then it will not be required for loading the module, so you can achieve exactly what you want, use local deps and still not include aws-sdk. claudia just needs to load the module to get the config out, it does not need to execute it.

@kyeotic

This comment has been minimized.

Copy link
Author

@kyeotic kyeotic commented Nov 8, 2016

I'm sure there are several solutions I as an app developer can implement to make a Lambda claudia-complaint. My point was that claudia should be able to handle any node lambda that runs in AWS. There shouldn't be claudia-specific constraints. Its your tool and you can decide that this case is too much work for Claudia to try to tackle.

WHY USE CLAUDIA?
AWS Lambda and API Gateway are incredibly flexible, but they can be tedious to set up, especially for simple scenarios. Running Node.js functions requires you to iron out quite a few quirks, that aren't exactly well documented. Claudia automates all those steps for you.

It's just a little surprising to read that, and then read that I need to write special code for an otherwise functional lambda to be deployed using Claudia.

@howardstevens

This comment has been minimized.

Copy link

@howardstevens howardstevens commented Mar 5, 2018

This has now become a real issue for me. Would you consider implementing a fix so that claudia can deploy to lambda without including AWS-SDK

KlaasH added a commit to azavea/tilegarden that referenced this issue Sep 27, 2018
The Lambda environment comes with the AWS SDK pre-installed, so it wastes
space in the bundle to install it ourselves.

Per [this issue](claudiajs/claudia#82), just moving
it to `devDependencies` won't work because Claudia validates the bundle and
it's an error for the package to be missing, but that can be worked around
by making at an optional dependency and telling Claudia not to install
optional.

Also bumps the version listed in package.json (and installed in development)
to the current version pre-installed on Lambda.

Resolves #118.
@vbichkovsky

This comment has been minimized.

Copy link

@vbichkovsky vbichkovsky commented Dec 20, 2019

Hello! Not sure if I'm doing it wrongly, but here is a use-case that should imho work differently:

mkdir test
cd test
nvm use 12.13.0
npm init -f
npm i -D claudia
npm i -S claudia-api-builder
npm i -O aws-sdk
npx claudia pack --no-optional-dependencies

Resulting zip file is 8.5Mb, in includes aws-sdk in node_modules.

Here is package.json for reference:

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "claudia": "^5.11.0"
  },
  "dependencies": {
    "claudia-api-builder": "^4.1.2"
  },
  "optionalDependencies": {
    "aws-sdk": "^2.594.0"
  }
}

I also noticed that there are two 'npm install' commands in the output, one without --no-optional.

Moreover, if I run

npx claudia pack --no-optional-dependencies --force

after that, the existing zipfile will be included in the new zipfile, resulting size is 17Mb.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Dec 20, 2019

@vbichkovsky can you please try wiping out your node_modules dir and then run

npm i --production --no-optional

and let me know if your node_modules has aws-sdk

after that, the existing zipfile will be included in the new zipfile, resulting size is 17Mb.

that's because you don't have a files section set in package.json, so claudia thinks the whole local directory needs to be packaged.

@vbichkovsky

This comment has been minimized.

Copy link

@vbichkovsky vbichkovsky commented Dec 23, 2019

@gojko node_modules does contain aws-sdk after wiping node_modules and running npm i --production --no-optional

Here is the list of directories in node_modules:

aws-sdk
base64-js
buffer
claudia-api-builder
events
ieee754
isarray
jmespath
punycode
querystring
sax
url
uuid
xml2js
xmlbuilder

I wonder if this use-case is reproducible on your box.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Dec 23, 2019

@vbichkovsky I think that's a bug with NPM then, it's not really in scope of Claudia to solve it. We have to rely on NPM to package the right dependencies. Perhaps submit that as a bug report there?

@JustFly1984

This comment has been minimized.

Copy link

@JustFly1984 JustFly1984 commented Dec 23, 2019

try yarn maybe instead of npm

@jandockx

This comment has been minimized.

Copy link

@jandockx jandockx commented Feb 6, 2020

  1. Put this as package.json in an empty directory
{
  "name": "claudiaawsexclude",
  "version": "0.0.0-experiment.0",
  "description": "Check whether claudia ignores aws-sdk",
  "optionalDependencies": {
    "aws-sdk": "^2.612.0"
  },
  "dependencies": {
    "js-yaml": "^3.13.1"
  },
  "devDependencies": {
    "claudia": "^5.12.0"
  }
}
  1. run npm install --no-optional: aws-sdk is not in node_modules

  2. start over, and pack

> rm -Rf node_modules/ package-lock.json
> npm install
> npx claudia pack --no-optional-dependencies

Inspect the .zip: aws-sdk is in node_modules

> node --version
v13.5.0
> npm --version
6.13.4
@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Feb 6, 2020

@jandockx can you please try npm install --production --no-optional instead of npm install --no-optional. my machine with your package actually includes claudia in node_modules, and aws_sdk within it, totally wrong. I still think it's a NPM bug.

@jandppw

This comment has been minimized.

Copy link

@jandppw jandppw commented Feb 6, 2020

> ls -al
total 24
drwxr-xr-x   4 jand  staff   128 Feb  6 15:41 .
drwxr-xr-x  19 jand  staff   608 Feb  6 09:15 ..
-rw-r--r--@  1 jand  staff  6148 Feb  6 09:41 .DS_Store
-rw-r--r--@  1 jand  staff   289 Feb  6 09:34 package.json
> npm install --production --no-optional
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN claudiaawsexclude@0.0.0-experiment.0 No repository field.
npm WARN claudiaawsexclude@0.0.0-experiment.0 No license field.

added 178 packages from 102 contributors and audited 358 packages in 5.097s
found 0 vulnerabilities

> ls -al node_modules/
total 0
drwxr-xr-x   8 jand  staff  256 Feb  6 15:41 .
drwxr-xr-x   6 jand  staff  192 Feb  6 15:41 ..
drwxr-xr-x   5 jand  staff  160 Feb  6 15:41 .bin
drwxr-xr-x   8 jand  staff  256 Feb  6 15:41 argparse
drwxr-xr-x   3 jand  staff   96 Feb  6 15:41 claudia
drwxr-xr-x   8 jand  staff  256 Feb  6 15:41 esprima
drwxr-xr-x  10 jand  staff  320 Feb  6 15:41 js-yaml
drwxr-xr-x  12 jand  staff  384 Feb  6 15:41 sprintf-js

Indeed, an unwanted claudia. But still no aws-sdk.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Feb 6, 2020

@jandppw aws-sdk is under claudia in this case, in claudia/node_modules

@jandppw

This comment has been minimized.

Copy link

@jandppw jandppw commented Feb 6, 2020

> ls -al node_modules/
total 0
drwxr-xr-x   8 jand  staff  256 Feb  6 16:24 .
drwxr-xr-x   6 jand  staff  192 Feb  6 16:24 ..
drwxr-xr-x   5 jand  staff  160 Feb  6 16:24 .bin
drwxr-xr-x   8 jand  staff  256 Feb  6 16:24 argparse
drwxr-xr-x   3 jand  staff   96 Feb  6 16:24 claudia
drwxr-xr-x   8 jand  staff  256 Feb  6 16:24 esprima
drwxr-xr-x  10 jand  staff  320 Feb  6 16:24 js-yaml
drwxr-xr-x  12 jand  staff  384 Feb  6 16:24 sprintf-js

> npm list js-yaml
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└── js-yaml@3.13.1 

> npm list sprintf-js
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└─┬ js-yaml@3.13.1
  └─┬ argparse@1.0.10
    └── sprintf-js@1.0.3 

> npm list argparse
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└─┬ js-yaml@3.13.1
  └── argparse@1.0.10 

> npm list esprima
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└─┬ js-yaml@3.13.1
  └── esprima@4.0.1 

> npm list claudia
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└── (empty)

> npm list aws-sdk
claudiaawsexclude@0.0.0-experiment.0 /Users/jand/Scratchpad/tmp/claudiaawsexclude
└── UNMET OPTIONAL DEPENDENCY aws-sdk@2.612.0 

I agree npm is doing incomprehensible stuff here. It should not have included claudia (but it didn't include aws-sdk).

node_modules/claudia/node_modules is quite large. But:

> ls -al node_modules/claudia/node_modules/aws-sdk
ls: node_modules/claudia/node_modules/aws-sdk: No such file or directory

BTW, claudia is also included with npm install --production, and then the aws-sdk is there too.

I don't see how this can result in claudia including aws-sdk in the package, but you know your code better, obviously.

@kandarpdave

This comment has been minimized.

Copy link

@kandarpdave kandarpdave commented Feb 7, 2020

I agree with @kyeotic here. For me, it's been a case with ioredis package, where Claudia tries to validate it , because it's trying to run the api on local. For it to work, in another tab, I've to stop/start Redis multiple times, until Claudia gets to go to the next step. This is extremely annoying, because even with me toggling Redis on/off, sometimes it takes a long time for Claudia to get to the next step.

The validation code is in this file: /Users/<username>/.nvm/versions/node/v13.2.0/lib/node_modules/claudia/src/tasks/validate-package.js (or similar) I'm just going to do return dir; at the top of the function to completely avoid package validation. @gojko Will doing so have any side-effects? Is there any other place where it tries to do validation? Both create and update use the same validate-package.js file, so I'm assuming this'll remove the validation part. Is that correct?

Claudia is great, but it really needs the skip-validation flag.

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Feb 7, 2020

@kandarpdave claudia can't work if it cannot load the api module, since the configuration of the api is in that module. validation itself is not so much of a problem, but the API module has to be instantiatable inside the context when claudia is deploying. I suggest lazy-loading the redis connection when you actually need it, and not in the global module config

@kandarpdave

This comment has been minimized.

Copy link

@kandarpdave kandarpdave commented Feb 7, 2020

Trying to understand why the API module has to be instantiatable. Can you explain? Why can't Claudia just deploy if the handler (and everything else needed for lambda) has been created already? It looks like the validation step "starts" the module. Would removing the step and having return dir; at the top of the function have any side-effects?

@kandarpdave

This comment has been minimized.

Copy link

@kandarpdave kandarpdave commented Feb 8, 2020

(Because someone else might want to figure out where claudiaJS is installed on their machine, run command -v claudia and it'll print a path possibly to the shortcut, and from there you can get to the original location where claudia is installed. On my another machine, it's at /Users/<username>/.config/yarn/global/node_modules/claudia/bin/cmd.js)

@gojko

This comment has been minimized.

Copy link
Member

@gojko gojko commented Feb 8, 2020

@kandarpdave api configuration (your routes, handler details etc) are in the module; claudia needs to load the module to read out the configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.