chewie icon indicating copy to clipboard operation
chewie copied to clipboard

Add a facade to abstract away calls to eventlet

Open samrussell opened this issue 7 years ago • 5 comments

Carrying on from the discussion in #70:

Changing put to put_nowait sounds like the correct thing to do in any case so I'd be happy to take that in. There's also an argument for making Chewie (and Beka, given the main message pumps are almost identical) be passed some sort of facade that means we can swap in a greenlet/coroutine system at runtime and nobody is any the wiser.

I'm in the middle of trying to decouple some of the radius code and for part of that I think I'm going to make a ChewieFactory, and that will probably inject an EAP socket, Radius socket, Radius lifecycle, and some sensible default for concurrency - that way the asyncio fork can be a one line change of from concurrency.asyncio import facade instead of from concurrency.greenlet import facade

What are your thoughts on this?

samrussell avatar Nov 16 '18 21:11 samrussell

It may be difficult to develop a framework that runs under both eventlet and asyncio. An alternative is to solidify the eventlet version, then develop the asyncio version as a downstream set of patches.

asyncio support relies on a syntactic change in the language to "mark" function calls that can "block" with async/await. The asyncio branch will likely have to patch any blocking call sites.

For example, here's the asyncio port of the send_eap_messages() method:

    async def send_eap_messages(self):
        """send eap messages to supplicant forever."""
        while self.running():
            await asyncio.sleep(0)
            message, src_mac, port_mac = await self.eap_output_messages.get()
            self.logger.info("Sending message %s from %s to %s" %
                             (message, str(port_mac), str(src_mac)))
            self.eap_socket.send(MessagePacker.ethernet_pack(message, port_mac, src_mac))

All the call sites that can block are explicitly marked with await. The await keyword can only be used in a function declared 'async def'.

byllyfish avatar Nov 18 '18 00:11 byllyfish

If the ChewieFactory let me specify the spawn/create_task function, I would use that in the asyncio port. Sometimes, it is useful to create "managed" tasks where you can control their lifespan and report unexpected exceptions.

byllyfish avatar Nov 18 '18 01:11 byllyfish

Chewie's external client API may be able to hide the greenlet/coroutine internals a little better. In faucet_dot1x.py, Chewie's run() api requires the client code to know how to spawn the eventlet. What if there was a "start()" api in Chewie that helped spawn the eventlet for run()? (The shutdown() method might be renamed "stop".)

byllyfish avatar Nov 18 '18 01:11 byllyfish

I've been thinking about ways to extract the loops from the actual message pump routines, and in actual fact none of them ever needs to do a sleep or a get or a put - they could all be changed a little to follow this sort of pattern:

# one iteration
def method(self, input):
  process_input(input)
  return output # not all of them

Then we could have another Scheduler class that holds and schedules them. The EapSocket/RadiusSocket needs to have eventlet/asyncio versions, and these classes can be injected into Chewie so it'll use the right one that matches the Scheduler.

We could then do one of a couple of things:

  • Poll all of the input sources (queues and Eap/Radius socket wrappers) and call the appropriate method when something is available
  • OR just replicate the current while/sleep setup

Could that work? That way any async calls are outside of Chewie, so it can be wrapped by anything that knows the API

samrussell avatar Nov 18 '18 20:11 samrussell

Yes, I think that inversion of control you are suggesting will work. This is kind of like a asyncio.Protocol, but with callbacks for radius and eap messages (e.g. on_radius_in, on_radius_out, ...)

On Sun, Nov 18, 2018 at 1:06 PM Sam Russell [email protected] wrote:

I've been thinking about ways to extract the loops from the actual message pump routines, and in actual fact none of them ever needs to do a sleep or a get or a put - they could all be changed a little to follow this sort of pattern:

one iteration

def method(self, input): process_input(input) return output # not all of them

Then we could have another Scheduler class that holds and schedules them. The EapSocket/RadiusSocket needs to have eventlet/asyncio versions, and these classes can be injected into Chewie so it'll use the right one that matches the Scheduler.

We could then do one of a couple of things:

  • Poll all of the input sources (queues and Eap/Radius socket wrappers) and call the appropriate method when something is available
  • OR just replicate the current while/sleep setup

Could that work? That way any async calls are outside of Chewie, so it can be wrapped by anything that knows the API

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/faucetsdn/chewie/issues/83#issuecomment-439721573, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGbr6ORVraHkesMvPwSbcdl3s-23IvIks5uwb3WgaJpZM4Ym_82 .

byllyfish avatar Nov 18 '18 20:11 byllyfish