Opened 8 years ago

Last modified 12 days ago

#3205 enhancement new

provide (and encourage) access to the reactor from every event source

Reported by: glyph Owned by:
Priority: high Milestone:
Component: core Keywords:
Cc: twm@… Branch:
Author:

Description

Currently if a user wants to access the reactor from a protocol's implementation, they must grab a reference to some global state "from twisted.internet import reactor".

However, it doesn't need to! It already has an object that is talking to reactor: its transport. The protocol should be able to do self.transport.reactor. (e.g) callLater(...).

Now, it just so happens that it can do exactly that; FileDescriptor has a reactor attribute. But it should be a part of the interface, and an encouraged idiom, and used consistently throughout Twisted.

I don't think that we can actually deprecate "from twisted.internet import reactor", but we should do it in a lot fewer places, since it makes testing unnecessarily tricky.

This ticket should probably be resolved when all ITransport implementations in Twisted properly provide this attribute, and it's defined and documented in the interface. However, we also need to file some more tickets for updating the protocol implementations to use it instead.

The same should be true of IDelayedCall, but it is perhaps less idiomatically useful, since IDelayedCall isn't provided to the functions that it's running.

Change History (7)

comment:1 Changed 7 years ago by glyph

  • Priority changed from normal to high

Bumping the priority, since this is something that keeps coming up.

comment:2 Changed 7 years ago by glyph

It also occurs to me that this ticket should deal with IListeningPort, IConnector, and perhaps the smattering of various datagram interfaces.

comment:3 Changed 5 years ago by <automation>

  • Owner glyph deleted

comment:4 follow-up: Changed 2 weeks ago by adiroiban

Can we add an attribute to ITransport without breaking compatibility ?

I would say that in this case the damage done by forcing users to update their ITransport implementations is minor.

The compatibility documentation is not clear about this. Maybe this is a good time to improve the compatibility documentation and take a decision regarding interfaces.

http://twistedmatrix.com/documents/current/core/development/policy/compatibility-policy.html#interface-changes


I think that we should also update the dev coding / user narrative documentation to discussion about using the reactor from the transport, as a best practice.

comment:5 in reply to: ↑ 4 Changed 2 weeks ago by glyph

Replying to adiroiban:

Can we add an attribute to ITransport without breaking compatibility ?

No. But, we can easily add it to a new interface, and then make all our ITransport implementations provide that interface; nothing incompatible about that.

I would say that in this case the damage done by forcing users to update their ITransport implementations is minor.

I'd rather just stick to the general policy of not adding stuff to interfaces. As it is, self.transport probably needs to provide half a dozen things - ITransport, ITCPTransport, IConsumer, IProducer, probably ISSLTransport too. Adding one more is therefore even more minor, and doesn't require a policy exception :).

The compatibility documentation is not clear about this. Maybe this is a good time to improve the compatibility documentation and take a decision regarding interfaces.

https://twistedmatrix.com/documents/current/core/development/policy/compatibility-policy.html#interface-changes

This could use a little more detail, but at least the core explanation is there. So, yes, we should do that, but it's an unrelated task.

I think that we should also update the dev coding / user narrative documentation to discussion about using the reactor from the transport, as a best practice.

Yes, that should be part of this ticket.

comment:6 Changed 2 weeks ago by twm

  • Cc twm@… added

comment:7 Changed 12 days ago by adiroiban

OK. I was not sure if creating an new interface for each new batch of attributes is an acceptable solution.

Regarding Amber's concern about passing the reactor in multi-reactor, I think that multi-reactor protocol can still be written to have a reactor passed at init time, rather than getting it from the transport... or mabye those protocol will have a makeGUIConnection() method in which the gui reactor is set.

Note: See TracTickets for help on using tickets.