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

refactor to more generic code #12

Merged
merged 4 commits into from
Oct 2, 2023
Merged

refactor to more generic code #12

merged 4 commits into from
Oct 2, 2023

Conversation

MoLow
Copy link
Member

@MoLow MoLow commented Sep 30, 2023

Ref: nodejs/release-cloudflare-worker#33

in order to get Cloudflare to link these soft links, I've refactored the code to be more generic, this way we can use in CF workers:

const { S3Client, ListObjectsV2Command } = require('@aws-sdk/client-s3')
const { fromIni } = require('@aws-sdk/credential-providers')
const { Linker } = require('nodejs-latest-linker/common.js')

const ENDPOINT = 'https://07be8d2fbc940503ca1be344714cb0d1.r2.cloudflarestorage.com'
const PROFILE = 'worker'
const BUCKET = 'dist-prod'
const RELEASE_DIR = 'nodejs/release/'
const DOCS_DIR = 'nodejs/docs/'

const client = new S3Client({
  endpoint: ENDPOINT,
  region: 'auto',
  credentials: fromIni({ profile: PROFILE })
});

(async function main () {
  const allDirs = await listDirectory(RELEASE_DIR)
  const linker = new Linker({ baseDir: RELEASE_DIR, docsDir: DOCS_DIR })
  const links = await linker.getLinks(allDirs, dir => listDirectory(`${dir}/`))
  console.log(links)
})()

async function listDirectory (dir) {
  let truncated = true
  let continuationToken
  let items = []
  while (truncated) {
    const data = await client.send(new ListObjectsV2Command({ Bucket: BUCKET, Delimiter: '/', Prefix: dir, ContinuationToken: continuationToken }))
    items = items.concat(data.CommonPrefixes ?? []).concat(data.Contents ?? [])
    truncated = data.IsTruncated
    continuationToken = data.NextContinuationToken
  }
  return items.map((d) => d.Prefix ?? d.Key)
}

@MoLow
Copy link
Member Author

MoLow commented Sep 30, 2023

CC @nodejs/build @nodejs/releasers

@MoLow
Copy link
Member Author

MoLow commented Sep 30, 2023

@@ -5,7 +5,7 @@ on:
branches: master

env:
NODE_VERSION: 12.x
NODE_VERSION: 18.x
Copy link
Member

Choose a reason for hiding this comment

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

We should keep testing on Node.js 12 until we can upgrade the version of Node.js used to run this tool on our server.

Copy link
Member Author

Choose a reason for hiding this comment

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

well, that breaks using modern javascript async/await ect :/
What is needed to upgrade node.js on the www server?

Copy link
Member

Choose a reason for hiding this comment

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

Current server is running Ubuntu 16.04 and using nodesource to install Node.js. I'm not sure whether Nodesource provide later versions of Node.js on Ubuntu 16.04. I'd like to update the OS on the server but it's risky given how dependent we are on that droplet at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

so we would either need to adjust this to run on node 12 or upgrade node without using nodesource, since upgrading the OS is not trivial
I prefer the second option, is that possible?

Copy link
Member

Choose a reason for hiding this comment

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

I think Node.js 18 is a non-starter on Ubuntu 16.04 due to glibc versions. Node.js 16 is a possibility -- there may even be a Nodesource distribution for it (I'd suggest testing in a separate Ubuntu 16.04 system first).

Copy link
Member

Choose a reason for hiding this comment

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

@MoLow Yes, it should be.

Copy link
Member

Choose a reason for hiding this comment

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

Based on the fact that https://deb.nodesource.com/node_16.x/dists/xenial/Release returns something, I think v16.x can be installed using the NodeSource distribution.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was able to install node16 on ubuntu 16.04 using nodesource. let me check these changes work on node 16

Copy link
Member Author

Choose a reason for hiding this comment

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

tests have passed. @richardlau should I go ahead and update www nodejs version?

Copy link
Member

Choose a reason for hiding this comment

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

@MoLow Yes (and keep an eye on tomorrow's nightly to check the indexer still runs ok). If you could also update https://github.com/nodejs/build/blob/b8b63ade1e76d8ccf669335ee384ed2ee34a3abe/ansible/www-standalone/tasks/base.yaml#L1-L3 (which says Node.js 8 but we are definitely running with 12 on the server) that would be great.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@MoLow MoLow merged commit 2f7ca14 into main Oct 2, 2023
1 check passed
@MoLow MoLow deleted the cloudflare branch October 2, 2023 19:35
@MoLow
Copy link
Member Author

MoLow commented Oct 2, 2023

I'v updated the www server to node 16. how does this package get published to npm?

@richardlau
Copy link
Member

I'v updated the www server to node 16. how does this package get published to npm?

ah I'll need to add you as a maintainer in npm. I've just invited you via npm's web ui -- if it needs to be done via the CLI (we had some issues adding someone to citgm in npm) I'll have to do that tomorrow.

@MoLow
Copy link
Member Author

MoLow commented Oct 3, 2023

richardlau added a commit to richardlau/node-1 that referenced this pull request Oct 19, 2023
A recent refactor of `nodejs-latest-linker` has moved the mapping
between Node.js versions and codenames to a different file.

Refs: nodejs/nodejs-latest-linker#12
nodejs-github-bot pushed a commit to nodejs/node that referenced this pull request Oct 21, 2023
A recent refactor of `nodejs-latest-linker` has moved the mapping
between Node.js versions and codenames to a different file.

Refs: nodejs/nodejs-latest-linker#12
PR-URL: #50299
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to nodejs/node that referenced this pull request Oct 23, 2023
A recent refactor of `nodejs-latest-linker` has moved the mapping
between Node.js versions and codenames to a different file.

Refs: nodejs/nodejs-latest-linker#12
PR-URL: #50299
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlau richardlau mentioned this pull request Oct 24, 2023
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
A recent refactor of `nodejs-latest-linker` has moved the mapping
between Node.js versions and codenames to a different file.

Refs: nodejs/nodejs-latest-linker#12
PR-URL: nodejs#50299
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to nodejs/node that referenced this pull request Nov 11, 2023
A recent refactor of `nodejs-latest-linker` has moved the mapping
between Node.js versions and codenames to a different file.

Refs: nodejs/nodejs-latest-linker#12
PR-URL: #50299
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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