Skip to content

feat!: deprecate node12. BREAKING CHANGE: Remove node12 from CI #1095

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

Merged
merged 10 commits into from
Feb 27, 2023

Conversation

ajewellamz
Copy link
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@ajewellamz ajewellamz requested a review from a team as a code owner February 17, 2023 23:00
@ajewellamz ajewellamz changed the title feat: update support policy for node12 feat!: deprecate node12. BREAKING CHANGE: Remove node12 from CI Feb 17, 2023
@texastony
Copy link
Contributor

texastony commented Feb 20, 2023

OK. 53c662b did not resolve the issue.
That was an attempt to roll back the CI libraries to the state they last succeeded in,
The Build Batch from #1043 4 months ago.

All the commands ran by testVectorsNodejsLatest passed until
npm run verdaccio-node-decrypt, when I hit an error accessing a KMS CMK,
presumable one that tonyknap-ToolsDevelopment does not have access to...

The CB is failing way before that, on:
npm run verdaccio-publish.

The error is:

lerna ERR! Error: Command failed with exit code 128: git rev-list --count 53c662b
lerna ERR! error: Could not read 7d43f32db69fd1b97540393430c947c3ee47c8cd
lerna ERR! fatal: Failed to traverse parents of commit 53c662be27fba33a8724d20b381b5b80a9949c45

Thinking about what Learna is doing and how CB is setup...
Learna may be trying to call the Git repo's commit history,
such that it can describe the changes between the last release and now.

The AWS CB Project only pulls 1 commit deep.
According to this stackoverflow post, CB does not create a .git dir... wild...

So we can use the --git-head parameter when invoking learna...
Can we do that?

UPDATE:
@ajewellamz I am not very JS smart... and trying to move fast on other things,
can you figure out how to:

Insert an if statement in util/local_verdaccio_publish,
such that IF the environmental variable CODEBUILD_RESOLVED_SOURCE_VERSION is set,
the args variable is updated to include --git-head ${CODEBUILD_RESOLVED_SOURCE_VERSION}.

(Link to args variable that should be conditionally updated.)

I will revert my change to get out of the way.

If that does not work,
we can increase the git clone depth of the CB project,
and that might resolve this learna error...

If those two do not work... ping the team channel and we will go from there.

@texastony
Copy link
Contributor

DO NOT MERGE THIS.
We want to do a minor release of the ESDK-JS,
supporting AWS-SDK V3,
before doing a Major Release that drops Node 12.

@texastony
Copy link
Contributor

(Will re-open after Minor Release).

@texastony texastony closed this Feb 23, 2023
@texastony texastony reopened this Feb 23, 2023
@texastony
Copy link
Contributor

texastony commented Feb 23, 2023

New challenge.
I tried to merge master into this branch,
and then update the package-lock,
and I cannot build node...

I will work on this...

Update @ 13:51 PST :: The issues are in kms-keyring-node.
That makes sense.

Had to update the package-lock and fix up
the webpack config in the karma.conf.js.
@texastony
Copy link
Contributor

Weird. 08fc26a failed AWS CB CI,
but only on testNodejsLatest, and it failed it in a weird way,
it just says it was Killed:

[Container] 2023/02/23 23:03:08 Running command npm run build
...<All normal messages>
> [email protected] build-node
> tsc -b tsconfig.json

Killed
ERROR: "build-node" exited with 137.

What Killed my build?

@texastony
Copy link
Contributor

Oi... I tried npm audit fix, thinking that maybe there was some dependency CB did not like and was killing.
But, a5872c2 fails testNodejsLatest and testNodejs14 with the weird Killed exception.
Maybe we are being throttled by CB?

I am going to put this down for now.
On my local, Node 14, Node 16, and Node 18 all pass npm ci; npm test.
We can pick this up another day,
I do not think this PR is the highest priority issue in my queue.

Copy link
Contributor

@seebees seebees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@texastony texastony merged commit b43fc0e into master Feb 27, 2023
@texastony texastony deleted the remove-node12-testing branch February 27, 2023 16:50
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.

3 participants