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

Add a facade to abstract away calls to eventlet #83

Open
samrussell opened this issue Nov 16, 2018 · 5 comments
Open

Add a facade to abstract away calls to eventlet #83

samrussell opened this issue Nov 16, 2018 · 5 comments

Comments

@samrussell
Copy link
Collaborator

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?

@byllyfish
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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".)

@samrussell
Copy link
Collaborator Author

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

@byllyfish
Copy link
Contributor

byllyfish commented Nov 18, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants