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

Simplify cancel_shielded_checkpoint ? #395

Closed
touilleMan opened this Issue Jan 9, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@touilleMan
Member

touilleMan commented Jan 9, 2018

I'm not sure what is the need for a mix of yield and asyncfunction decorator here:

def asyncfunction(fn):
# Set the coroutine flag
fn = types.coroutine(fn)
# Then wrap it in an 'async def', to enable the "coroutine was not
# awaited" warning
@wraps(fn)
async def wrapper(*args, **kwargs):
return await fn(*args, **kwargs)
return wrapper
# This class object is used as a singleton.
# Not exported in the trio._core namespace, but imported directly by _run.
class CancelShieldedCheckpoint:
pass
@asyncfunction
def cancel_shielded_checkpoint():
"""Introduce a schedule point, but not a cancel point.
This is *not* a :ref:`checkpoint <checkpoints>`, but it is half of a
checkpoint, and when combined with :func:`checkpoint_if_cancelled` it can
make a full checkpoint.
Equivalent to (but potentially more efficient than)::
with trio.open_cancel_scope(shield=True):
await trio.hazmat.checkpoint()
"""
return (yield CancelShieldedCheckpoint).unwrap()

From what I understand, it is because an awaited object must be of a type with an __await__ method.
But in such a case there could be a much simpler wait to do this:

class CancelShieldedCheckpointType:
    def __await__(self):
        yield self
CancelShieldedCheckpoint = CancelShieldedCheckpoint()
del CancelShieldedCheckpointType

async def cancel_shielded_checkpoint():
    return await CancelShieldedCheckpoint

This way CancelShieldedCheckpointType is just a helper class (hence destroyed as soon at it is no longer needed) and CancelShieldedCheckpoint is still a singleton, this time compatible with await syntax.

@njsmith

This comment has been minimized.

Member

njsmith commented Jan 10, 2018

Hmm, you're right, that code does look kinda overcomplicated. I'd rather not create __await__ methods just for this, but what do you think of #397?

@touilleMan

This comment has been minimized.

Member

touilleMan commented Jan 10, 2018

Looks good to me 👍

@njsmith

This comment has been minimized.

Member

njsmith commented Jan 10, 2018

Want to click merge, then? :-)

@touilleMan

This comment has been minimized.

Member

touilleMan commented Jan 10, 2018

That would be an honor !

@njsmith

This comment has been minimized.

Member

njsmith commented Jan 10, 2018

Thanks! BTW, sorry I haven't followed up with pytest-trio stuff recently -- I'm hoping to get back to that soon, maybe this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment