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

Jump to bottom

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 only 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 only 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

    @asvetlov asvetlov closed this as completed Apr 1, 2022
    @asvetlov asvetlov closed this as completed Apr 1, 2022
    @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

    @bedevere-bot bedevere-bot mentioned this issue Jul 29, 2022
    Merged
    @neonene neonene mentioned this issue Jul 29, 2022
    Closed
    ambv added a commit that referenced this issue Aug 4, 2022
    @ambv
    gh-91323: Revert "Allow overriding a future compliance check in async…
    0342c93 
    …io.Task (GH-32197)" (GH-95442)

This reverts commit d4bb38f.
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 4, 2022
    @ambv @miss-islington
    pythongh-91323: Revert "Allow overriding a future compliance check in…
    5202170 
    … asyncio.Task (pythonGH-32197)" (pythonGH-95442)

This reverts commit d4bb38f.
(cherry picked from commit 0342c93)

Co-authored-by: Łukasz Langa <lukasz@langa.pl>
    @bedevere-bot bedevere-bot mentioned this issue Aug 4, 2022
    Merged
    ambv added a commit that referenced this issue Aug 4, 2022
    @miss-islington @ambv
    gh-91323: Revert "Allow overriding a future compliance check in async…
    5ac3d0f 
    …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
    Assignees
    No one assigned
    Labels
    3.11 only security fixes topic-asyncio
    Projects
    asyncio
    Status: Done
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @1st1 @asvetlov @graingert @kumaraditya303