-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Currently, the way we configure events is to put an annotation @intersect_event() on functions which are part of the stack which can emit events. The INTERSECT-SDK attempts to enforce schema conformity via checking the call stack for a function decorated with @intersect_event. We were also able to include the information about the function which was called in the event itself if we needed to.
This original approach was misguided for a few reasons:
- At no point do we really need to care what function the event was emitted from, the capability can log this if they want but the SDK and INTERSECT event consumers do not need to care.
- Users can reuse the same event definitions across multiple functions, but have to make sure they get their
IntersectEventDefinitionconfiguration consistent across all of them. This makes the decorator pointless and weirdly interdependent on other decorators. - Calling
inspect.stack()and then traveling the stack in reverse in practice only needs to pass through one function (and sometimes a couple more) but is still completely unnecessary. - This leads to a somewhat confusing API which heavily restricts event emission to occurring inside the capability itself. Consider the following scenario:
class MyCapability(IntersectBaseCapabilityImplementation):
# ... etc.
@intersect_event(events={'increment_counter': IntersectEventDefinition(event_type=int)})
def event_emitter_root(self):
self.emit_event(self.counter)
def emit_event(self, amount: int):
self.intersect_sdk_emit_event('increment_counter', amount)
@intersect_event(events={'increment_counter': IntersectEventDefinition(event_type=int)})
def event_emitter_decorated(self):
self.intersect_sdk_emit_event('increment_counter', amount)
capability = MyCapability()
def some_function(value: int):
# The next line will NOT work!!! There is no @intersect_event decorated function in the stack!
capability.emit_event(value)
# The next line does work
capability.emit_event_decorated(value)This is not intuitive at all. We also have to update both decorators if we ever decide to change the response type, or even if we decide to switch emitting onto MINIO instead of sending the data through the message broker.
Proposed solution
While the event representation event_name_string:key to IntersectEventDefinition:value mapping is fundamentally sound, and represents itself well in schema (disregarding the other issues of #23 and #13 , which are not a problem for the SDK public-facing API itself but are internally-managed issues), the very concept of a decorator for events seems to be dubious.
We are already using class variables to define intersect_sdk_capability_name; when we generate the schema and the message routes, we only look at the class variables and do not care if the user changes them per object . I think that we should just make intersect_sdk_capability_event_config a class variable; it can be a dictionary of event names to IntersectEventDefinitions, as it's already represented internally.
So we should replace all @intersect_event decorators with this:
class MyCapability(IntersectBaseCapabilityImplementation):
intersect_sdk_capability_name = 'MyCapability'
intersect_sdk_capability_event_config = {
'increment_counter': IntersectEventDefinition(event_type=int),
'timely_image_generator': IntersectEventDefinition(event_type=bytes, content_type='image/png', data_handler=IntersectDataHandler.MINIO)
}
def __init__(self):
# ... impl
def emit_event(self, amount: int):
self.intersect_sdk_emit_event('increment_counter', amount)Now it does not matter where self.intersect_sdk_emit_event is called, you just have to make sure that the event key and the event value in the function arguments match up with what you declared on the class variable. You also only have to update a decorator in one location if you want to change information about the event.