Skip to content

Commit

Permalink
Update ingest module for node 20
Browse files Browse the repository at this point in the history
This update has a couple of controversial changes in it:

Updating got to v14 means we're using a pure ESM module given sindre's
stance on not supporting common exports.  That's being done due to
incompatible changes in node streams requires at least got v12

Additionally there's a probable outstanding issue in got
sindresorhus/got#2341 related to node v20/fs
readstreams/nock and/or msw incompatibility (as well as a possible
open source contrib)
  • Loading branch information
Jkovarik committed Apr 23, 2024
1 parent 83d06dd commit 10490c7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/ingest/package.json
Expand Up @@ -50,7 +50,7 @@
"cksum": "^1.3.0",
"encodeurl": "^1.0.2",
"fs-extra": "^5.0.0",
"got": "^14.0.0",
"got": "^14.2.0",
"is-ip": "^2.0.0",
"is-valid-hostname": "^0.1.1",
"jsftp": "https://github.com/jkovarik/jsftp.git#add_288",
Expand Down
22 changes: 10 additions & 12 deletions packages/ingest/src/HttpProviderClient.js
Expand Up @@ -4,11 +4,11 @@ const fs = require('fs');
const https = require('https');
const isIp = require('is-ip');
const { basename } = require('path');
const { pipeline } = require('node:stream/promises');
const { pipeline } = require('stream/promises');
const { PassThrough } = require('stream');
const Crawler = require('simplecrawler');
const { CookieJar } = require('tough-cookie');
const { promisify } = require('util');
const stream = require('node:stream');

const {
buildS3Uri,
Expand Down Expand Up @@ -214,8 +214,7 @@ class HttpProviderClient {
async download(params) {
// This doesn't work because typescript transpilation
// const { got } = await import('got');

// This does though! This is fine.
//
// eslint-disable-next-line no-eval
const { default: got } = await eval('import("got")');
const { remotePath, localPath } = params;
Expand Down Expand Up @@ -260,8 +259,7 @@ class HttpProviderClient {
async sync(params) {
// This doesn't work because typescript transpilation
// const { got } = await import('got');

// This does though! This is fine.
//
// eslint-disable-next-line no-eval
const { default: got } = await eval('import("got")');
const { destinationBucket, destinationKey, fileRemotePath } = params;
Expand Down Expand Up @@ -312,15 +310,15 @@ class HttpProviderClient {
* @returns {Promise<string>} the uri of the uploaded file
*/
async upload(params) {
// This doesn't work because typescript transpilation
// This doesn't work due to typescript transpilation
// const { got } = await import('got');

// This does though! This is fine.
//
// eslint-disable-next-line no-eval
const { default: got } = await eval('import("got")');

const { localPath, uploadPath } = params;
log.info(params);
const { readStream = fs.createReadStream(localPath) } = params.test || {};
log.info({ localPath, uploadPath });
await this.setUpGotOptions();
await this.downloadTLSCertificate();
const options = {
Expand All @@ -334,9 +332,9 @@ class HttpProviderClient {
const remoteUrl = buildURL(options);
got.stream.options = options;
await pipeline(
fs.createReadStream(localPath),
readStream,
got.stream.post(remoteUrl),
new stream.PassThrough()
new PassThrough()
);

log.info(`Finishing uploading ${localPath} to ${remoteUrl}`);
Expand Down
12 changes: 11 additions & 1 deletion packages/ingest/test/test-HttpProviderClient.js
Expand Up @@ -324,6 +324,16 @@ test.serial('upload() attempts to upload a file', async (t) => {
.post(path.join('/', uploadPath))
.reply(200);

await httpProviderClient.upload({ localPath, uploadPath });
// This text fixture is a workaround to an ongoing issue with
// got/pipeline/msw & nock in node 20. Integration tests
// must cover the full use case
const readStream = new Readable({
read(item) {
this.push(JSON.stringify(item));
},
});
readStream.push('foobar');
readStream.push(null);
await httpProviderClient.upload({ localPath, uploadPath, test: { readStream } });
t.true(nock.isDone());
});

0 comments on commit 10490c7

Please sign in to comment.