Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncio.Controller #74486

Closed
warsaw opened this issue May 7, 2017 · 17 comments
Closed

asyncio.Controller #74486

warsaw opened this issue May 7, 2017 · 17 comments
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir

Comments

@warsaw
Copy link
Member

warsaw commented May 7, 2017

BPO 30300
Nosy @warsaw, @pitrou, @vstinner, @giampaolo, @njsmith, @1st1
PRs
  • bpo-30300: A convenient "controller" for asyncio-based servers #1492
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-02-03.14:48:37.075>
    created_at = <Date 2017-05-07.18:24:57.358>
    labels = ['3.7', 'library']
    title = 'asyncio.Controller'
    updated_at = <Date 2018-02-03.14:48:37.073>
    user = 'https://github.com/warsaw'

    bugs.python.org fields:

    activity = <Date 2018-02-03.14:48:37.073>
    actor = 'barry'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-02-03.14:48:37.075>
    closer = 'barry'
    components = ['Library (Lib)']
    creation = <Date 2017-05-07.18:24:57.358>
    creator = 'barry'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30300
    keywords = []
    message_count = 17.0
    messages = ['293205', '293260', '293264', '293464', '293465', '294638', '294666', '294675', '294698', '294725', '294767', '294790', '294794', '294798', '294801', '294803', '311558']
    nosy_count = 6.0
    nosy_names = ['barry', 'pitrou', 'vstinner', 'giampaolo.rodola', 'njs', 'yselivanov']
    pr_nums = ['1492']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue30300'
    versions = ['Python 3.7']

    @warsaw
    Copy link
    Member Author

    warsaw commented May 7, 2017

    Over in https://github.com/aio-libs/aiosmtpd we have a Controller class which is very handy for testing and other cases. I realized that this isn't really aiosmtpd specific, and with just a few tweaks it could be appropriate for the stdlib.

    I have a branch ready for a pull request. This is the tracking/discussion issue.

    @warsaw warsaw added 3.7 (EOL) end of life stdlib Python modules in the Lib dir labels May 7, 2017
    @njsmith
    Copy link
    Contributor

    njsmith commented May 8, 2017

    Looks interesting! What's the advantage over running the server and the test in the same loop? The ability to use blocking operations in the tests, and to re-use an expensive-to-start server over multiple tests? (I've mostly used threads in tests to run blocking code for interoperability testing, and kept the async code in the main thread, so this is a bit novel to me.)

    @warsaw
    Copy link
    Member Author

    warsaw commented May 9, 2017

    On May 08, 2017, at 11:06 PM, Nathaniel Smith wrote:

    Looks interesting! What's the advantage over running the server and the test
    in the same loop? The ability to use blocking operations in the tests, and to
    re-use an expensive-to-start server over multiple tests?

    So, the ability to re-use expensive-to-start servers is definitely one of the
    advantages. I use nose2's layers, but test fixtures would fall into the same
    category.

    As for running the server and tests in the same loop; I haven't tried that,
    but it seems like it would be more complicated to set up (maybe that's
    dependent on the code under test). More important is that I want to block the
    tests until the server starts up. I'm not sure (haven't tried) whether that's
    possible when running them all in the same loop.

    One other use case I have is for the LMTP server in Mailman 3. The controller
    turns out to be useful based on the start/stop framework for MM3 "runners".
    That's probably strictly doable without the controller, but it's convenient,
    readable, and a nice reuse.

    @vstinner
    Copy link
    Member

    I'm not sure that Controller is generic enough to be part of asyncio.

    I'm not sure about the cancellation of all pending tasks on stop().

    Why not starting by putting this class in a library to mature its API?

    @warsaw
    Copy link
    Member Author

    warsaw commented May 11, 2017

    On May 11, 2017, at 12:09 AM, STINNER Victor wrote:

    Why not starting by putting this class in a library to mature its API?

    It's already part of aiosmtpd although not with the small amount of
    generic-ness included here. It's been useful and stable. So this is just
    refactoring out the aiosmtpd-ness of it. It doesn't seem big enough to put
    into yet another separate library.

    @pitrou
    Copy link
    Member

    pitrou commented May 28, 2017

    I think the API is too specific. Instead of requiring hostname and port, why not let the user override setup and teardown coroutines?

    In your case, this could be:

    async def setup(self):
        self.server = await self.loop.create_server(...)
    
    async def teardown(self):
        await self.server.wait_closed()

    @warsaw
    Copy link
    Member Author

    warsaw commented May 29, 2017

    Hi Antoine,

    On May 28, 2017, at 11:07 AM, Antoine Pitrou wrote:

    I think the API is too specific.

    Can you elaborate? What's too specific about it? Do you have in mind a use
    case where you wouldn't need to provide hostname and port?

    Instead of requiring hostname and port, why not let the user override setup
    and teardown coroutines?

    In your case, this could be:

    async def setup(self):
    self.server = await self.loop.create_server(...)

    async def teardown(self):
    await self.server.wait_closed()

    It's certainly possible to factor those out so they could be overridden, I'm
    just not sure why that's needed.

    @pitrou
    Copy link
    Member

    pitrou commented May 29, 2017

    Can you elaborate? What's too specific about it? Do you have in mind a use case where you wouldn't need to provide hostname and port?

    Any use case where setup is more elaborate than calling create_server(...). For example I might write a UDP server. Or a distributed system that listens to several ports at once, or launches a thread pool. etc.

    @warsaw
    Copy link
    Member Author

    warsaw commented May 29, 2017

    On May 29, 2017, at 07:07 AM, Antoine Pitrou wrote:

    For example I might write a UDP server. Or a distributed system that listens
    to several ports at once, or launches a thread pool. etc.

    Thanks, those are nice motivational examples.

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2017

    I'm not sure we want this to be in asyncio: it's a very high-level object somewhere in between the low-level and the application level. Some things off the top of my head that users will want from this API:

    • detailed logging or hooks to implement it
    • hooks on thread start / stop
    • coroutines to run before starting the server
    • coroutines to run before stopping the loop
    • custom undhandled exceptions handlers
    • type of the server created: TCP/UDP/Unix
    • ability to configure SSL
    • etc

    Since asyncio is no longer provisional, it won't be possible to evolve the API in bugfix releases, which will likely make it impossible to use for many users until 3.8.

    In general, the advice for things like this is to put them on PyPI, gather some feedback, and sort out the API details.

    -1 to add this in 3.7 in its current state.

    P.S. It would be interesting to try to evolve the idea a bit further: it would be interesting if Controller was a high-level description of a service and we had a separate concept of ControllerRunner to run Controllers in threads, processes and corotuines. Maybe build something like erlang supervisor trees out of them.

    @warsaw
    Copy link
    Member Author

    warsaw commented May 30, 2017

    On May 29, 2017, at 11:42 PM, Yury Selivanov wrote:

    • detailed logging or hooks to implement it
    • hooks on thread start / stop
    • coroutines to run before starting the server
    • coroutines to run before stopping the loop
    • custom undhandled exceptions handlers
    • type of the server created: TCP/UDP/Unix
    • ability to configure SSL
    • etc

    P.S. It would be interesting to try to evolve the idea a bit further: it
    would be interesting if Controller was a high-level description of a service
    and we had a separate concept of ControllerRunner to run Controllers in
    threads, processes and corotuines. Maybe build something like erlang
    supervisor trees out of them.

    There's also value in doing one simple thing that adds convenience for users.
    I don't personally have any interest in building something as elaborate as the
    above, but I've used the simple idea here several times in different projects.

    @vstinner
    Copy link
    Member

    Barry: would you be ok to start by adding Controller to asyncio.test_utils,
    and wait later to expose it in the public API?

    @1st1
    Copy link
    Member

    1st1 commented May 30, 2017

    STINNER Victor added the comment:

    Barry: would you be ok to start by adding Controller to asyncio.test_utils,
    and wait later to expose it in the public API?

    Sorry, but we are going to deprecate and remove test_utils soon. It's a bunch of internal unit test helpers used privately by asyncio. They are not documented and not supported. Now that asyncio repo is in CPython repo and we don't release it on its own, i see no reason to keep test_utils (we can move it to 'Lib/test'.

    Again, the natural way of something like Controller to end up in asyncio is to either go through full PEP process, or live some time on PyPI and prove to be useful.

    @1st1
    Copy link
    Member

    1st1 commented May 30, 2017

    There's also value in doing one simple thing that adds convenience for users.
    I don't personally have any interest in building something as elaborate as the
    above, but I've used the simple idea here several times in different projects.

    Yeah, I understand. I totally see how Controller can be useful, moreover,
    I've implemented something like it bunch of times (usually for running
    tests). But with Guido no longer actively involved with asyncio, and me
    being the only one who is actively working on/supporting asyncio, I
    think we need to go through the full PEP process when we want to add
    new APIs. bpo simply does not provide enough exposure: guys from
    Twisted and Tornado, for instance, won't see this discussion.

    I'd be glad to assist you with the PEP though!

    @warsaw
    Copy link
    Member Author

    warsaw commented May 30, 2017

    On May 30, 2017, at 10:36 PM, Yury Selivanov wrote:

    Again, the natural way of something like Controller to end up in asyncio is
    to either go through full PEP process, or live some time on PyPI and prove to
    be useful.

    A PEP feels like overkill; we don't require PEPs for every addition to an
    existing stdlib module or package. My worry too is that a PEP tends to evoke
    endless bikeshedding.

    I appreciate that you want to be careful not to saddle asyncio with too much
    baggage, or that you don't feel Controller is significant enough to generalize
    and put in the package. Perhaps a middle ground would be to label parts of
    the asyncio API provisional, and add Controller to that?

    @1st1
    Copy link
    Member

    1st1 commented May 31, 2017

    I appreciate that you want to be careful not to saddle asyncio with too much
    baggage, or that you don't feel Controller is significant enough to generalize
    and put in the package. Perhaps a middle ground would be to label parts of
    the asyncio API provisional, and add Controller to that?

    Thing is, when asyncio was provisional, we still couldn't significantly
    change it or break it. Never in asyncio stdlib era had we removed or
    redesigned some APIs. Only small additions and bug fixes. And honestly,
    maintaining something provisional and changing it in bugfix releases
    is too much stress: we managed to break loop.connect_socket once
    because nobody tests bugfix RCs. It was broken for ~6 months.

    IMHO: the design of Controller is currently incomplete (see one of my
    previous comments). Even in this thread two other core devs raised a
    question that the API isn't generic enough to be part of asyncio.
    Right now it's not flexible and tailored for one specific use case.
    Should the user need slightly more, they will have to copy/paste it,
    or, worse, inherit from it and use its private APIs.

    @warsaw
    Copy link
    Member Author

    warsaw commented Feb 3, 2018

    There doesn't seem to be much appetite for this in the stdlib, so closing. It'll just have to live in a third party module.

    @warsaw warsaw closed this as completed Feb 3, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants