-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add support for connector upgrades #51
Conversation
e0990e8
to
d9c4cc1
Compare
- name: Get npm package version | ||
id: get-npm-package-version | ||
run: | | ||
PACKAGE_VERSION=`npm version | sed -rn "2 s/.*: '([^']*)'.*/\1/g; 2 p"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the version of? The npm
binary itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or does this return the version field from package.json
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets the version field from package.json.
|
||
RUN apk add jq curl | ||
|
||
COPY /docker /scripts | ||
RUN : "${CONNECTOR_VERSION:?Connector version must be set}" | ||
RUN echo ${CONNECTOR_VERSION} > /scripts/CONNECTOR_VERSION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we bake in the version.
docker/upgrade-connector.sh
Outdated
rm -rf /tmp/connector-upgrade | ||
mkdir /tmp/connector-upgrade | ||
|
||
# We copy the package.json and package-lock.json to a temporary directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a thought about this, did you try running npm install --package-lock-only
? https://docs.npmjs.com/cli/v11/commands/npm-install#package-lock-only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I suppose you'd still need to do the cat
stuff below to change the package.*
files without messing up their perms)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good suggestion, I didn't know about that flag. I'll give it a shot, it's worth it even if it just improves performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I can avoid all the copying and cat stuff with --package-lock-only
. npm doesn't change the permissions on the files and doesn't modify node_modules. Good call dude!
This PR adds support for connector upgrades via the forthcoming
ddn connector upgrade
command.This works by having the
ddn
CLI run the docker image with the connector directory mounted and running the new/scripts/upgrade-connector.sh
script. This script copies the user's package.json and package-lock.json files and performs an npm install on them to upgrade the SDK package. It then overwrites their files with the updated versions.Fixes ENG-1119