Use `async for` for iterations

[asvetlov]

Right now many APIs look like

for image in (await docker.images.list()):
        print(f" {image['Id']} {image['RepoTags'][0] if image['RepoTags'] else ''}")

That's work and might be useful sometimes but I consider it ugly. Better notation should be:

async for image in docker.images.list():
        print(f" {image['Id']} {image['RepoTags'][0] if image['RepoTags'] else ''}")

It's more native and doesn't require extra parenthesis.

Implementation should support both ways, sure. It could be done by returning Context class from docker.images.list() non-coroutine call. The context should support __await__ and __aiter__ for returning an async iterator with __anext__.

It's not very hard task but I have no time for it now unfortunately. @achimnol would you do it? I'll be happy to review if needed.

[achimnol]

Sorry for late response. I've been busy with my company work. I strongly agree with your idea.

[barrachri]

Something like this?

class DockerImages(object):
    def __init__(self, docker):
        self.docker = docker

    async def list(self, **params) -> Dict:
        """
        List of images
        """
        response = await self.docker._query_json(
            "images/json", "GET",
            params=params,
            headers={"content-type": "application/json", },
        )
        return AsyncContext(response)

class AsyncContext:
    def __init__(self, coro):
        self._coro = coro

   def __await__(self):
        return self._coro.__await__()

    def __aiter__(self):
        return self

    async def __anext__(self):
        data = await self._coro()
        if data:
            return data
        else:
            raise StopAsyncIteration

[jettify]

I am not sure it is right use case for async for since images = await docker.images.list() returns list of images eagerly all at once. But fetching logs is other story, since we may need to wait on new line to appear for some time. Right now we have:

stream_output = await shell_container.log(stdout=True, follow=True)
async for line in stream_output:
    print(line)

should be something like:

async for line in shell_container.log(stdout=True, follow=True):
    print(line)

[barrachri]

I think it was a syntactic sugar to avoid

for image in (await docker.images.list()):

or

stream_output = await shell_container.log(stdout=True, follow=True)
async for line in stream_output:

[jettify]

yeah

for image in (await docker.images.list()):
    print(image)

could be

images = await docker.images.list()
for image in images:
    print(image)

I find this variant more idiomatic, since we can apply list comprehensions on images, and it should be faster since we probably yield control to loop less frequently.

[asvetlov]

Second example looks good and perfectly understandable but adding a sugar for getting rid of one line and one temporary variable looks attractive, many people will use it.

[cecton]

I'm not 100% sure you really want to convert a future of a list to an async generator. It's 2 different things to me and should be handled differently. Imagine I want to store in a variable the list of the containers or the images, with the async for syntax I'm gonna need to make an async list comprehension to convert the list explicitly.

I'm pretty sure in Docker we need async generators for some really useful examples like the events list which is a continuous stream of dict that you need to wait for every item to popup but not a fixed list of containers or a fixed list of Docker images imho.

I do like making things simpler but here it just doesn't sound right to me, the type of result is different: if the dev asks for a stream, you get an async generator, if not, you get an awaitable with the complete result. I also think it's good for the developer who is going to use the library to understand what he is doing when he asks for a stream and what is the expected result that makes sense.

I'm not in favor of this change.

[jettify]

Agreed. Lets do not rush with introduction of this API and handle logging and events first.

[asvetlov]

operations like list() are single calls, I've decided to keep them untouched. On the other hand streaming operations, e.g. docker.images.pull(..., stream=True) starting from aiodocker 0.16 require async for notation. Non-streaming mode still supports plain await.

The reason for the change is resource lifecycle control: HTTP response must be closed before returning from the call or on async iterator exhausting.