Skip to content

Conversation

@TomasTomecek
Copy link
Contributor

Needed for: d11wtq/dockerpty#30
Transitively for: docker/compose#2023

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does follow the pattern established by attach()/attach_socket() but I wonder if it could be just another kwarg like stream?

What do you think @shin- ?

def exec_start(self, exec_id, detach=False, tty=False, stream=False, socket=False):
    ...
    func = get_response_handler(stream, socket)
    return func(res)

def get_response_handler(stream, socket):
    if stream and socket:
        raise ValueError(...)

    if stream:
        return functools.partial(self._get_result_tty, True)
    if socket:
        return  self._get_raw_response_socket
    return  functools.partial(self._get_result_tty, False)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. To be honest, I actually vote for socket=False since there is already an option for asking for stream. On top of it, you'll get this feature for free in future (for new similar API calls).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shin- waiting for your reply before I start rewriting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, socket as a param makes a lot of sense. 👍

@TomasTomecek TomasTomecek force-pushed the improve-exec-api branch 2 times, most recently from f81634e to 1ad6132 Compare November 23, 2015 17:58
@TomasTomecek TomasTomecek mentioned this pull request Nov 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants