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

Jobs.downloadSingleArtifactFile can no longer be piped #338

Closed
biw-joelschou opened this issue Jun 6, 2019 · 10 comments
Closed

Jobs.downloadSingleArtifactFile can no longer be piped #338

biw-joelschou opened this issue Jun 6, 2019 · 10 comments

Comments

@biw-joelschou
Copy link

Description

With v4.5.1 we'd been using Jobs.downloadSingleArtifactFile and node-unzipper to extract artifacts files like this:

        const artifactRequest = await glProjects.Jobs.downloadSingleArtifactFile(
            project.id,
            job.id,
            '',
            {
                stream: true
            }
        );

        await new Promise( ( resolve, reject ) => {
            artifactRequest.pipe(
                unzip.Extract( { path: targetDir } )
                    .on( 'close', () => {
                        resolve();
                    } )
                    .on( 'error', err => {
                        reject( err );
                    } )
            );
            artifactRequest.on( 'error', ( err ) => {
                reject( err );
            } );
        } );

This was working splendidly. Since updating to v6.0.0 it no longer works. I get this error:

TypeError: artifactRequest.pipe is not a function

Upon digging in and doing quite a bit of debugging, this appears to be a result of the change to ky from got. The streamed response structure has changed completely, from a full-on GL Request object like this:

Request {
  _events: [Object: null prototype] { pipe: [Function] },
  _eventsCount: 1,
  _maxListeners: undefined,
  headers:
   { 'private-token': 'snipped',
     host: 'snipped',
     accept: 'application/json' },
  qsStringifyOptions: { arrayFormat: 'brackets' },
  resolveWithFullResponse: false,
  rejectUnauthorized: true,
  method: 'GET',
  readable: true,
  writable: true,
  explicitMethod: true,

// ...

To this:

{ body:
   'PK\u0003\ /* ... */ u0000\u0000',
  headers:
   { 'accept-ranges': 'bytes',
     connection: 'close',
     'content-length': '460278',
     'content-type': 'application/zip',
     date: 'Thu, 06 Jun 2019 14:18:45 GMT',
     etag: '"snipped"',
     'last-modified': 'Tue, 04 Jun 2019 20:03:17 GMT',
     server: 'AmazonS3',
     'x-amz-id-2':
      'snipped',
     'x-amz-request-id': 'snipped' },
  status: 200 }

I've tried various things to get the zip written out and opened, but it's just not cooperating. Any ideas?

@jdalrymple
Copy link
Owner

jdalrymple commented Jun 6, 2019

hmm, not off the top of my head :( but i can tinker with it this weekend. I've been meaning to fix some streaming problems in #297.

I did make a modification to ky to allow for streaming functionality but its possible im not handling it correctly. The area where this should happen is in the KyRequester. I think the response variable would be a stream, but id have to verify!

@jdalrymple jdalrymple added this to the 6.x.x milestone Jun 6, 2019
@jdalrymple jdalrymple added the Bug label Jun 6, 2019
@biw-joelschou
Copy link
Author

Slightly embarrassing confession: I was using downloadSingleArtifactFile incorrectly. When I pass in a path to the file I want (the empty string argument in my top example), I do get back the file I expect in the body of the response.

That said, I changed over to downloadLatestArtifactFile thinking I would get a pipeable response containing the ZIP. Turns out, no. The response is the same as the example at the end of my initial comment and I still can't write out the ZIP file in the body of the response.


Two other things:

  1. I'm wondering if this is a change on the GL side of the API?
  2. I did notice your addition to ky to support streams, but you didn't seem to implement their onProgress function as something a user can hook into. I have no idea if that would solve anything, but it was something I noticed.

@jdalrymple
Copy link
Owner

🤣 Yea I noticed that as well, i think i just implemented it incorrectly. Maybe if i fix that returnable streams will be possible? Let me try that first and see what comes back.

@jdalrymple
Copy link
Owner

I tinkered with this over the weekend but kept running into road blocks locally. Ill try and devise another way of figuring this out later this week. Tonight is a write off #WeTheNorth 😂

@joelschou
Copy link

Thanks for looking into it! Go 🦖!

@jdalrymple
Copy link
Owner

I think i implemented the streaming capability incorrectly, and/or its not supported how i believed it was. I
I'm asking the creator of ky for more info :)

@jdalrymple
Copy link
Owner

jdalrymple commented Jun 17, 2019

So apparently the streaming functionality isn't supported by ky, but could be with a PR. So I may fork it and submit a PR with the functionality. I'm the mean time ill remove it from the exposed API to avoid confusion :(

sindresorhus/ky#3

@jdalrymple jdalrymple removed this from the 6.x.x milestone Jun 18, 2019
@jdalrymple jdalrymple added Enhancement and removed Bug labels Aug 6, 2019
@arsdehnel
Copy link
Contributor

Is there a reason you're using ky for this? It seems like that's for browsers and this package seems more aimed at node runtimes. I tried the below with got (from the same folks as ky but aimed for node runtimes) and it worked fine.

await pipeline(
    got( 'https://gitlab.example.com/api/v4/projects/<project-id>/jobs/artifacts/master/download?job=my-job-name', {
        stream: true,
        headers: {
            'PRIVATE-TOKEN': process.env.GITLAB_API_TOKEN
        }
    } ),
    fs.createWriteStream( path.resolve( targetDir, 'test.zip' ) )
);

If there is a reason for using ky over got then I guess that works. I know I can use whatever requestor I want but just curious about a switch to got.

@jdalrymple
Copy link
Owner

jdalrymple commented Sep 5, 2019

The only reason we moved to ky/universal-ky was the promise of a cross platform solution which seemed to not be fully the case (especially with regards to the streaming support). The main options are to:

  1. Finish adding complete streaming support to Ky, which should be possible since its built off the Fetch API
  2. Create a GotRequester that is used for the non browser builds and have two different functionalities for browser and node.

I REALLY don't want to do 2. since it would result in a split of support for the library, but i also don't have the time to tackle 1. :/

@jdalrymple
Copy link
Owner

jdalrymple commented Mar 22, 2020

Should be pipeable now with the stream option. See latest release @gitbeaker/node

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

No branches or pull requests

4 participants