blazzy/podman-rest-client

Exporting a container via libpod doesn't return a stream of the tarball from the response body

Closed this issue · 7 comments

Function:
https://docs.rs/podman-rest-client/latest/src/podman_rest_client/v5/apis/containers.rs.html#400-419

According to libpod API docs, this request returns (hopefully by streaming) the entire tarball in its response body, but the return type of this mapped function is (), which seems incorrect.

Annoyingly the official swagger spec says it produces an application/json response.

    /libpod/containers/{name}/export:
        get:
            description: Export the contents of a container as a tarball.
            operationId: ContainerExportLibpod
            parameters:
                - description: the name or ID of the container
                  in: path
                  name: name
                  required: true
                  type: string
            produces:
                - application/json
            responses:
                "200":
                    description: tarball is returned in body

I was hoping to punt issues like this one as I didn't need these endpoints just yet. I think it should be straightforward to fix though. By modifying both the swagger file and the api generator to support application/x-tar. Might have something out today or tomorrow.

@kanpov

How does v0.12.1 look? I figure treating it as a patch release is okay as the functions were not unusable otherwise. Doc and test.

The only two functions that have been updated in this release are image_export_libpod and container_export_libpod. There are likely more that need the exact same adjustment. They should be really quick to update once spotted.

@kanpov

How does v0.12.1 look? I figure treating it as a patch release is okay as the functions were not unusable otherwise. Doc and test.

The only two functions that have been updated in this release are image_export_libpod and container_export_libpod. There are likely more that need the exact same adjustment. They should be really quick to update once spotted.

There's one big problem: since you've decided to box the Stream, could it be Send? Otherwise, there are extremely annoying problems when trying to actually use the Stream due to it being !Send. If you just did impl Stream with RPITIT like bollard and didn't box the Stream (because why box it honestly?), this wouldn't be an issue in the first place though.

So, until this is fixed, I still have to resort to forking the podman binary with the export CLI arguments instead of using podman-rest-client.

This is basically following the client generation pattern that OpenAPITools generator project is using for its hyper client. With its traits for each sub grouping of the api. This started life using that generator. The generator which funnily enough also didn't have it's functions returning futures with Send constraints. You'll see me wondering there why they bother using traits like that in the PR.

Making it impl Stream would mean the library user would have to import the individual traits for v5::apis::Containers, v5::apis::Images and so on. This seemed a little too onerous. I'd rather just drop the traits and intermediate functions altogether. Which is something I'm considering and open to feedback on. There shouldn't be any name collisions as every operation has it's own unique name in the swagger file.

v0.12.3 is up with the Send constraint.

let handle = tokio::spawn(async move {
let stream = client
.containers()
.container_export_libpod("podman_rest_client_stream_thread_test");

v0.12.3 is up with the Send constraint.

let handle = tokio::spawn(async move {
let stream = client
.containers()
.container_export_libpod("podman_rest_client_stream_thread_test");

Thanks for the effort! I'm not against pin-boxing the return types, as long as the Send trait bound is there. v0.12.3 works just fine.

Forgot to close the issue!