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

Add support for connector upgrades #51

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

daniel-chambers
Copy link
Collaborator

@daniel-chambers daniel-chambers commented Jan 21, 2025

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

@daniel-chambers daniel-chambers self-assigned this Jan 21, 2025
@daniel-chambers daniel-chambers force-pushed the daniel/connector-upgrade-support branch from e0990e8 to d9c4cc1 Compare January 21, 2025 04:26
- name: Get npm package version
id: get-npm-package-version
run: |
PACKAGE_VERSION=`npm version | sed -rn "2 s/.*: '([^']*)'.*/\1/g; 2 p"`
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Collaborator Author

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
Copy link
Contributor

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.

rm -rf /tmp/connector-upgrade
mkdir /tmp/connector-upgrade

# We copy the package.json and package-lock.json to a temporary directory
Copy link
Contributor

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

Copy link
Contributor

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)

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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!

@daniel-chambers daniel-chambers merged commit 08ba057 into main Jan 22, 2025
4 checks passed
@daniel-chambers daniel-chambers deleted the daniel/connector-upgrade-support branch January 22, 2025 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants