search

jfhbrook / pyee

A rough port of Node.js's EventEmitter to Python with a few tricks of its own
https://github.com/jfhbrook/public
MIT License
371 stars 39 forks source link

Namespace support #67

Closed HeavenVolkoff closed 4 years ago

HeavenVolkoff commented 4 years ago

Hello,

First off, thanks for the lib. I recently stumbled upon it and began using in some of my projects. It has worked like a charm. However, in the latest project I am working on, the need for namespaces in events has emerged. So, I decided to take a shot at implementing it.

I wanted to generate the least impact possible at the overall architecture of BaseEventEmitter while implementing this feature. So, what I ended up with was using tuples to represent namespaces and their levels. Basically I implemented a subclass: NamespaceEventEmitter, where (emit|on|once|...) receives a tuple that represents the namespaced event, instead of a string (It converts strings to single-element tuples for ease of use). Also, It has a modified _call_handlers method that calls all listeners in the namespace from the most generic level to the level the emitted event specified. Additionally, I implemented namespaced versions of each available emitter type. All the changes are self-contained to pyee.namespace as not to break any current behavior.

I implemented some tests and added the namespaced versions of each specific emitter to the tests they currently have implemented, everything is passing. However, I only ran them in python3.8, so some more testing is needed.

Solves https://github.com/jfhbrook/pyee/issues/47

jfhbrook commented 4 years ago

Oh wow, I did not expect this!

Namespaces are a feature I've resisted for a long time. The common wisdom in the node world is that there are usually alternate ways of solving the problem that will end up with a more performant architecture anyway.

All that said, you are not the first person to ask about namespaces and at least one person has forked pyee to hack in namespace support, and you put in all this work so I want to take this suggestion seriously.

I'd like to know more about your use case. What are you working on, and how does this change help solve it? Please be as specific as you can be without giving away state secrets, even if you think the details of your app are boring.

On my end, I want to read this more closely and roll around what API I think we would want. Personally, I would rather use a string-based DSL rather than a tuple, possibly using a glob-like DSL for pattern matching. I also want to think about whether I want this functionality to be pulled in via mixin, concrete implementation, class decorator, instance munging (like in uplift) or some combination thereof.

Finally: Were you using black to format these files? A few of these changes appear to be formatting-only. I'd rather not mix formatting changes with this PR, especially if it makes the formatting inconsistent. I am however down to merge black support. https://github.com/jfhbrook/db_hooks is a project of mine that uses black which you should be able to crib off of.

Thanks for the PR!

HeavenVolkoff commented 4 years ago

Thanks for giving this a look.

As for my use case, at work, I am responsible for maintaining a custom RPC protocol developed in-house with python and asyncio that is used by my team to implement some AI microservices. Currently, I am upgrading the protocol's capabilities to collect metrics and metadata to solve some complaints made by the operations team that is responsible for maintaining these services. My current approach is to implement an event stack that will provide an easy way to emit the new metrics during the protocol pipeline and a simple API to register custom metrics. However, to do this I will need to rework some parts of the internal protocol's pipeline into events and listeners themselves. Most of this internal logic is currently implemented as a bunch of Futures with callbacks attached that get executed when the parsing phase of each package type finishes and resolves them. I am going to replace this with an emit call at the end of each package type parse, and rework the previously callback logic into event listeners. However, there are some cases in which there is common logic applied across a group of similar packages. I know I could implement this by simply emitting both a package specific event and another event for the group logic, albeit cumbersome to do/maintain (especially in more complex cases). That said, what appears to be the most straight forward way to implement this behavior is to implement namespaced events.

So that is about it. I initially though about doing a string-based DSL for the namespace, but ultimately I gave up on the idea because it seemed to be lot more complex than my use case justified.

About the formatting, that was a bit of laziness on my part. I did a global search/replace to add the Namespaced classes to the existing test cases, so I use black to fix up the formatting. I am going to revert those changes.

Also, I noticed that the tests are failing in python2, I will have a look at what is happening there. Quite some time I don't use python 2