search

redsift / sandbox-javascript

DAG Container for NodeJS
1 stars 0 forks source link

Move capnp install steps in package.json #6

Closed cv711 closed 4 years ago

cv711 commented 4 years ago

This allows for a simple installation flow when it's used as a submodule (e.g. grip + SDK) The only drawback is the usage of the --unsafe-perm flag inside the Dockerfile.

deepakprabhakara commented 4 years ago

Not sure this is essential, given the use of --unsafe-perm. Might as well run the script where needed and leave instructions in hilt.

cv711 commented 4 years ago

By adding all the logic in package.json we don't need alternative mechanisms for installation.

Another approach to this, since we are moving forward with making node 12 our default version, we could update the capnp dependency in package.json as "capnp":"github.com/redsift/node-capnp#node12" and just change that for the other sandbox versions we support. This way there is no need for a separate installation script and no need for the --unsafe-perm flag.

deepakprabhakara commented 4 years ago

@cv711 That's better, only this repo will need changes to support node10 and node8.

cv711 commented 4 years ago

The usage of --unsafe-perm is limited inside the container and is not needed for local installation. Following the second approach of installing capnp as a npm dep directly from Github is failing during installation, so we are leaving it as it is.