-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for connector upgrades #51
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
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.
|
|
||
| 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 upgradecommand.This works by having the
ddnCLI run the docker image with the connector directory mounted and running the new/scripts/upgrade-connector.shscript. 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