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

Allow overriding future-task compliance check in asyncio #91323

Closed
asvetlov opened this issue Mar 30, 2022 · 5 comments
Closed

Allow overriding future-task compliance check in asyncio #91323

asvetlov opened this issue Mar 30, 2022 · 5 comments
Labels
3.11 bug and security fixes topic-asyncio

Comments

@asvetlov
Copy link
Contributor

BPO 47167
Nosy @asvetlov, @1st1
PRs
  • bpo-47167: Allow overriding a future compliance check in asyncio.Task #32197
  • 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 = None
    created_at = <Date 2022-03-30.15:34:50.023>
    labels = ['3.11', 'expert-asyncio']
    title = 'Allow overriding future-task compliance check in  asyncio'
    updated_at = <Date 2022-04-01.02:14:55.054>
    user = 'https://github.com/asvetlov'

    bugs.python.org fields:

    activity = <Date 2022-04-01.02:14:55.054>
    actor = 'yselivanov'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['asyncio']
    creation = <Date 2022-03-30.15:34:50.023>
    creator = 'asvetlov'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 47167
    keywords = ['patch']
    message_count = 3.0
    messages = ['416374', '416467', '416468']
    nosy_count = 2.0
    nosy_names = ['asvetlov', 'yselivanov']
    pr_nums = ['32197']
    priority = 'normal'
    resolution = None
    stage = 'resolved'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue47167'
    versions = ['Python 3.11']

    @asvetlov
    Copy link
    Contributor Author

    Now asyncio.Task has a strict hardcoded check for futures processes on task's step.
    Sometimes third-party library [1] wants to replace it with custom logic.
    Currently it is impossible without implementing the full asyncio.Task replacement from scratch.
    asyncio.Task is a non-trivial highly optimized part of async, keeping a third-party version in sync with the reference implementation takes more time and effort than it should be.

    This issue proposes adding Task._check_future(future) method that could be overridden in subclasses. The default C implementation uses a fast path that has no performance penalty.

    1. https://github.com/aio-libs/aioloop-proxy implements nested loop proxies that are very useful for writing tests. A proxy can work in parallel with ancestors but checks all own acquired resources cleanup on
      the proxy closing.

    @asvetlov asvetlov added 3.11 bug and security fixes topic-asyncio labels Mar 30, 2022
    @asvetlov
    Copy link
    Contributor Author

    asvetlov commented Apr 1, 2022

    New changeset d4bb38f by Andrew Svetlov in branch 'main':
    bpo-47167: Allow overriding a future compliance check in asyncio.Task (GH-32197)
    d4bb38f

    @1st1
    Copy link
    Member

    1st1 commented Apr 1, 2022

    Sorry, I don't like the committed change and I think it should be reverted.

    Futures and Tasks have a documented get_loop() method which simply should be called by Task machinery, without the need to expose (and document) private methods.

    @1st1 1st1 reopened this Apr 1, 2022
    @1st1 1st1 reopened this Apr 1, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @graingert
    Copy link
    Contributor

    This patch isn't needed to support the aioloop-proxy use case, you can use an interceptor coroutine to pass the check instead:

    aio-libs/aioloop-proxy@a655652

    this way aioloop proxy works with asyncio.Task CTask PyTask and in addition custom set_task_factory Tasks

    ambv added a commit that referenced this issue Aug 4, 2022
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 4, 2022
    … asyncio.Task (pythonGH-32197)" (pythonGH-95442)
    
    This reverts commit d4bb38f.
    (cherry picked from commit 0342c93)
    
    Co-authored-by: Łukasz Langa <lukasz@langa.pl>
    ambv added a commit that referenced this issue Aug 4, 2022
    …io.Task (GH-32197)" (GH-95442) (GH-95652)
    
    This reverts commit d4bb38f.
    (cherry picked from commit 0342c93)
    
    Co-authored-by: Łukasz Langa <lukasz@langa.pl>
    @kumaraditya303
    Copy link
    Contributor

    Closing as the PR was reverted and it is not needed #91323 (comment).

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 bug and security fixes topic-asyncio
    Projects
    Status: Done
    Development

    No branches or pull requests

    4 participants