Suggestion for API improvement

achimnol commented 7 years ago

Let's provide more "pythonic" API using magic methods.

For instance, if we use __getitem__, we could do:

Current:

img_id = '...'
image = docker.images.image(img_id)
await image.do_something()

cid = '...'
container = docker.containers.container(cid)
await container.do_something()

Suggested:

img_id = '...'
image = docker.images[img_id]
await image.do_something()

cid = '...'
container = docker.containers[cid]
await container.do_something()

Note: there is no "DockerImage" (not "DockerImages") class yet. The above code is just an illustration.

achimnol commented 7 years ago

To resolve the container/image information on the fly, we need to use some trick(?) to make __getitem__() asynchronous. Here is the example code.

asvetlov commented 7 years ago

-1 Making __getitem__ async changes semantic and looks very confusing.

barrachri commented 7 years ago

The current api implementation looks clean enough to me.

Are there more reasons apart from it could be more pythonic?

achimnol commented 7 years ago

Currently DockerContainer class is relying on an internal dictionary to store and provide the properties of a Docker container. As I have reported in #100, the content of this internal dict is changed unexpectedly upon certain API calls.

I'd like to make a consistent view of Docker objects with typed properties, while still allowing low-level access to original data retrieved by the API for both backward and forward-compatibility.

To achieve this, we need a common asynchronous API of retrieving Docker objects from their collections (e.g., DockerImages -> DockerImage, DockerContainers -> DockerContainer, DockerVolumes -> DockerVolume, etc.). I thought async __getitem__ could be one of the way and might look "Pythonic" because it reuses the item-getter interface of generic collections. Of course, if you think it's better to keep the original synchronous semantic of __getitem__, it is also fine for me to define a separate method like get() – that was just an idea.

achimnol commented 7 years ago

@asvetlov BTW, apart from this issue, what do you think of having something like in __agetitem__() in the Python core language?

barrachri commented 7 years ago

PEPit :D

asvetlov commented 7 years ago

I don't think we need __agetitem__. Think about await mapping['a']['b']. Event more, where to put await for __asetitem__?

Hmm, we already have await docker.images.get(name). @achimnol how a proposed new method is differ from it?

barrachri commented 6 years ago

Closed because it has been inactive for many days.

Feel free to reopen this issue in case 👍

aio-libs / aiodocker

Suggestion for API improvement #144