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: C implemeted Future cause Tornado test fail #72616

Closed
methane opened this issue Oct 13, 2016 · 9 comments
Closed

asyncio: C implemeted Future cause Tornado test fail #72616

methane opened this issue Oct 13, 2016 · 9 comments
Assignees
Labels
3.7 (EOL) end of life topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@methane
Copy link
Member

methane commented Oct 13, 2016

BPO 28430
Nosy @gvanrossum, @terryjreedy, @methane, @socketpair, @1st1
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • future-iter-send.patch
  • 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 = 'https://github.com/methane'
    closed_at = <Date 2016-10-25.10:31:07.366>
    created_at = <Date 2016-10-13.12:43:15.175>
    labels = ['type-bug', '3.7', 'expert-asyncio']
    title = 'asyncio: C implemeted Future cause Tornado test fail'
    updated_at = <Date 2017-03-31.16:36:16.006>
    user = 'https://github.com/methane'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:16.006>
    actor = 'dstufft'
    assignee = 'methane'
    closed = True
    closed_date = <Date 2016-10-25.10:31:07.366>
    closer = 'methane'
    components = ['asyncio']
    creation = <Date 2016-10-13.12:43:15.175>
    creator = 'methane'
    dependencies = []
    files = ['45165']
    hgrepos = []
    issue_num = 28430
    keywords = ['patch']
    message_count = 9.0
    messages = ['278569', '278686', '279095', '279104', '279150', '279382', '279388', '279404', '279406']
    nosy_count = 6.0
    nosy_names = ['gvanrossum', 'terry.reedy', 'methane', 'socketpair', 'python-dev', 'yselivanov']
    pr_nums = ['552']
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28430'
    versions = ['Python 3.6', 'Python 3.7']

    @methane
    Copy link
    Member Author

    methane commented Oct 13, 2016

    @methane methane added the 3.7 (EOL) end of life label Oct 13, 2016
    @methane methane self-assigned this Oct 13, 2016
    @methane methane added topic-asyncio type-bug An unexpected behavior, bug, or error labels Oct 13, 2016
    @methane
    Copy link
    Member Author

    methane commented Oct 15, 2016

    Another test failure is reported.

    I have setup a 3.6 build on Travis of our project GNS3 and it seem to fail.
    https://travis-ci.org/GNS3/gns3-server/builds/167703118

    @methane
    Copy link
    Member Author

    methane commented Oct 21, 2016

    pure Python Future.__iter__ don't use what yield-ed.

        def __iter__(self):
            if not self.done():
                self._asyncio_future_blocking = True
                yield self  # This tells Task to wait for completion.
            assert self.done(), "yield from wasn't used with future"
            return self.result()  # May raise too.

    I felt no-None value is sent by iter.send(val) wasn't make sense.
    But Tornado did it (maybe, for compatibility to Tornado's generator.)

    So this patch ignores when non-None value is passed.
    Additionally, I moved NEWS entry about C Future from "core and builtin"
    section to "library" section.

    @methane
    Copy link
    Member Author

    methane commented Oct 21, 2016

    Test failure in GNS3 seems not relating to this.
    It may be fixed via bpo-28492, maybe.

    @1st1
    Copy link
    Member

    1st1 commented Oct 21, 2016

    The patch looks good. 2 things:

    • It appears it also touches Misc/NEWS a bit too much. Please make sure to not to commit that.

    • I'd also add a comment explaining why we ignore values passed to FI.send() and simply send None. The reason is how Future.__iter__ is designed:

       class Future:
          def __iter__(self):
            if not self.done():
                self._asyncio_future_blocking = True
                yield self  # This tells Task to wait for completion.
            assert self.done(), "yield from wasn't used with future"
            return self.result()  # May raise too.

    ^-- Future.__iter__ doesn't care about values that are pushed to the generator, it just returns "self.result()".

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 25, 2016

    New changeset b471447352ac by INADA Naoki in branch '3.6':
    Issue bpo-28430: Fix iterator of C implemented asyncio.Future doesn't
    https://hg.python.org/cpython/rev/b471447352ac

    New changeset bd141ec2973a by INADA Naoki in branch 'default':
    Issue bpo-28430: Fix iterator of C implemented asyncio.Future doesn't
    https://hg.python.org/cpython/rev/bd141ec2973a

    @methane
    Copy link
    Member Author

    methane commented Oct 25, 2016

    • It appears it also touches Misc/NEWS a bit too much. Please make sure to not to commit that.

    I wont to move move this entry from "Core and Builtins" section to "Library" section:

    • Issue bpo-26081: Added C implementation of asyncio.Future.
      Original patch by Yury Selivanov.

    May I do it?

    @methane methane closed this as completed Oct 25, 2016
    @terryjreedy
    Copy link
    Member

    +- Issue bpo-28430: Fix iterator of C implemented asyncio.Future doesn't accept

    + non-None value is passed to it.send(val).

    This NEWS entry is not a coherent English sentence. Please revise. I don't understand it well enough to suggest a revision, but would review a replacement.

    @methane
    Copy link
    Member Author

    methane commented Oct 25, 2016

    I'm sorry about my bad English.

    Fix iterator of C implemented asyncio.Future

    This meant:

    fut = asyncio.Future()  # C implemented version of asyncio.Future
    it = iter(fut)  # Iterator of it
    it.send(42)     # raised TypeError before. It was not compatible with Python version of asyncio.Future

    @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 topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants