architect-examples / arc-example-persist-data

Example persisting data with DynamoDB and .arc
https://arc.codes/guides/data
4 stars 1 forks source link

Add `hydrate -- install` capability to run `npm install` on each Function #3

Open pmuellr opened 5 years ago

pmuellr commented 5 years ago

following the directions in the readme, the hydrate command ended up spewing a bunch of errors:

$ npx hydrate
        app ⌁ testdata
     region ⌁ us-east-1
    profile ⌁ default
    version ⌁ 4.5.6

Error
Error: npm ci --ignore-scripts in src/http/get-index exited with code 1
  npm ERR! cipm can only install packages with an existing package-lock.json or npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or later to generate it, then try again.

  npm ERR! A complete log of this run can be found in:
  npm ERR!     /Users/pmuellr/.npm/_logs/2019-03-28T12_19_12_096Z-debug.log

Error: npm ci --ignore-scripts in src/http/get-notes-000noteID exited with code 1
  npm ERR! cipm can only install packages with an existing package-lock.json or npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or later to generate it, then try again.

... above repeated a number of times

It seemed like it was complaining about the package.json files in the src/http directories, so I ran the following to npm install in there:

$ ls src/http | xargs -I % sh -c 'cd src/http/%; npm install'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN testapp-get-index@ No description
npm WARN testapp-get-index@ No repository field.
npm WARN testapp-get-index@ No license field.

added 29 packages from 21 contributors and audited 32 packages in 4.175s
found 0 vulnerabilities

... repeated a number of times

and voilà!

$ npx hydrate
        app ⌁ testdata
     region ⌁ us-east-1
    profile ⌁ default
    version ⌁ 4.5.6

✓ Success! Installed all dependencies in 6078ms

I guess my suggestion would be that there should be a post-install script in the root package.json file, that should arrange to call npm install for all the subdirs.

ryanblock commented 5 years ago

Hey @pmuellr! A normal npm i that generates a lockfile is exactly what happens when you create your project: https://github.com/arc-repos/architect/blob/master/src/create/aws/_install-workflows-and-data.js#L10-L11

Not sure how you got in that state, please provide steps to reproduce.

Related, looks like you're a major behind. It's possible (even likely) we've fixed bugs in there since 4.5.x, so I'd suggest upgrading to 5.5.x.

pmuellr commented 5 years ago

repro is just following the readme in this repo:

https://github.com/arc-repos/arc-example-persist-data/blob/3e326b26d98bd3ac7824030a52d5908ecd79c90c/readme.md#L3-L6

Of course I did a cd arc-example-persist-data after the git clone :-)

I tried updating the deps:

  "dependencies": {
    "@architect/architect": "^5.5.10",
    "@architect/data": "^2.0.15",
    "nodemon": "^1.18.10"
  }

Didn't seem to help.

The problem seems to be the directories src/http/* need to have a package-lock.json in them, for npm ci to work, so I made that happen with the command:

ls src/http | xargs -I % sh -c 'cd src/http/%; npm install'

After that, npx hydrate works fine.

Perhaps npm install needs to be used instead of npm ci, or some options can be passed to npm ci to let it survive without a package-lock.json file ...

Also, for reference:

$ npm -v
6.4.1

$ node -v
v10.15.1
ryanblock commented 5 years ago

Ah, I was wondering how you got an Arc project without lockfiles! There's the bug: https://github.com/arc-repos/arc-example-persist-data/blob/master/.gitignore#L2

We standardize around npm ci to keep your dev env as close to the real thing as possible, and ci requires lockfiles to operate. Thus, lockfiles must always to be checked into repos (which is NPM's best practice anyway), so this was on us. Sorry about that!

That said, I've been thinking about adding an npx hydrate --install action anyway, so this is probably a good occasion to do so.

Can you rescope this issue to be an enhancement request for npx hydrate --install and I'll knock that out?

pmuellr commented 5 years ago

Assuming npx hydrate --install will do an install vs ci - seems right to me! Or I wonder if npx hydrate should peek to see if there's a package-lock.json and decide on running install vs ci itself. Option free!

In terms of "rescoping", I can just change the title of the issue, but not sure if you were wanting a separate issue open. Feel free to change the title yourself, otherwise I can create a new issue as a feature request ...

ryanblock commented 5 years ago

I've debated auto-npm installing if no package-lock is found, but papering over those errors would encourage bad habits (like not checking in the lockfile, which would then enable per-environment dependency drift). I think an explicit install action is probably the way to go.