cloudconvert / cloudconvert-node

CloudConvert node.js SDK
https://cloudconvert.com/api/v2
Other
164 stars 41 forks source link

wip #112

Open KnorpelSenf opened 8 months ago

KnorpelSenf commented 8 months ago

Closes #111.

KnorpelSenf commented 8 months ago

not tested at all, does not even compile yet

kingmesal commented 5 months ago

I'm very interested in dumping axios in favor of the built-in fetch. I run a lot of things in cloudflare workers and this would be quite a helpful change.

Any update on when this might be ready?

KnorpelSenf commented 5 months ago

I'm ready to continue with this, I was just hoping for pointers regarding https://github.com/cloudconvert/cloudconvert-node/issues/111#issuecomment-1952311108 before I go ahead. Shipping a custom implementation of multipart/form-data clearly gives us the best library, but I didn't feel like deciding alone whether or not we want to accept that in this library.

josiasmontag commented 5 months ago

Sorry for the delay. We should go with a custom multipart/form-data implementation then as @KnorpelSenf suggested.

I think we need to keep the optional size parameter in the upload() method though. If one passes a custom stream with unknown length, there is no way to calculate a correct Content-Length header.

KnorpelSenf commented 5 months ago

Sorry for the delay. We should go with a custom multipart/form-data implementation then as @KnorpelSenf suggested.

Alright, I will commence my work on this in the coming time, but my time for OSS is a bit limited right now, so please don't expect an immediate implementation :)

I think we need to keep the optional size parameter in the upload() method though. If one passes a custom stream with unknown length, there is no way to calculate a correct Content-Length header.

What if somebody passes a read stream, or an AsyncIterator<Uint8Array>? For example, if people download a file from one server via fetch and then obtain the file stream from the response body, there is no way of knowing the file size in advance. Is the content-length header strictly required?

josiasmontag commented 5 months ago

What if somebody passes a read stream, or an AsyncIterator<Uint8Array>? For example, if people download a file from one server via fetch and then obtain the file stream from the response body, there is no way of knowing the file size in advance. Is the content-length header strictly required?

Unfortunately, S3 strictly requires the Content-Length header for uploads. Therefore, I would like to automatically detect the file size if possible (file streams) and otherwise use a size parameter for custom streams.

In the mentioned example the server should return a Content-Length header which could be passed as size parameter to the upload() method.

KnorpelSenf commented 5 months ago

What if somebody passes a read stream, or an AsyncIterator<Uint8Array>? For example, if people download a file from one server via fetch and then obtain the file stream from the response body, there is no way of knowing the file size in advance. Is the content-length header strictly required?

Unfortunately, S3 strictly requires the Content-Length header for uploads. Therefore, I would like to automatically detect the file size if possible (file streams) and otherwise use a size parameter for custom streams.

Alright!

In the mentioned example the server should return a Content-Length header which could be passed as size parameter to the upload() method.

Yeah I'll add this to the doc string

dncnbuck commented 4 months ago

Hi @KnorpelSenf - I see you've mentioned you won't be able to dedicate much time to this PR - but regardless I'm going to ask! Do you have a rough timeframe here?

KnorpelSenf commented 4 months ago

Yep, I'm gonna give a talk at https://events.geekle.us/typescript24/ on Tuesday so there are a number of things to prepare until then. The rest of that week will be spent catching up with life in general, and the week after that I'm mostly free and looking forward to getting back to this (but no promises, life can be surprising and I don't want to set myself deadlines)

KnorpelSenf commented 4 months ago

If you feel like contributing, you can check out the base implementation of what I'll do here in this file: https://github.com/grammyjs/grammY/blob/main/src/core/payload.ts

KnorpelSenf commented 2 months ago

I will get back to this after my summer vacation, likely in September. Sorry for the delays, there were a few other important tasks on my agenda :)