Custom Event Hooks Event Emission #164
Conversation
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
|
Thanks for the contribution! It looks like @achinthalapalli is an internal user so signing the CLA is not required. However, we need to confirm this. |
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
ajaycj
changed the title
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
django_declarative_apis/events.py
Outdated
| ) | ||
|
|
||
| try: | ||
| module = __import__(module_name, fromlist=[func_name]) |
There was a problem hiding this comment.
importlib is recommended rather than using __import__ directly. there are a couple other example usages in the library if you search for importlib.
django_declarative_apis/events.py
Outdated
| try: | ||
| module = __import__(module_name, fromlist=[func_name]) | ||
| return getattr(module, func_name) | ||
| except ImportError as e: |
There was a problem hiding this comment.
any reason to not just let these bubble up as is?
There was a problem hiding this comment.
Agreed. Addressed in b5866c6
| TASK_RETRY = "task_runner:retry" | ||
|
|
||
|
|
||
| def _import_hook(hook_path): |
There was a problem hiding this comment.
should this also check that the path is a callable? an example of that can be found here.
django_declarative_apis/events.py
Outdated
|
|
||
| def emit_events(event_type, payload): | ||
| """ | ||
| Emit a metric event using the configured hook and New Relic if configured. |
There was a problem hiding this comment.
This docstring should be updated and have newrelic removed.
django_declarative_apis/events.py
Outdated
| metric_type (str): Type of the metric (from MetricType enum). | ||
| payload (dict): The data associated with the metric. | ||
| """ | ||
| hook = getattr(settings, "DDA_EVENT_HOOK", None) |
There was a problem hiding this comment.
can't this and the next few lines be evaluated at import-time, or cached internally once executed the first time? are you intentionally importing on every execution?
There was a problem hiding this comment.
Addressed in b5866c6
django_declarative_apis/events.py
Outdated
| except Exception as e: | ||
| logger.error(f"Error in custom hook for events: {e}", exc_info=True) | ||
|
|
||
| if newrelic_agent: |
There was a problem hiding this comment.
newrelic should probably just be pulled from dda entirely. it was only used here and it can be added to our project repo instead of being here.
| }, | ||
| ) | ||
| emit_events( | ||
| EventType.QUEUE_LENGTH.value, |
There was a problem hiding this comment.
this is a little odd. the current value of the enum is the value expected by newrelic schema. if we're removing that from the library, i'd expect emit_events to take an enum member rather than the concrete value. that way, callers can use the enum members directly for internal mapping rather than value. not merge blocking though.
There was a problem hiding this comment.
also, the event name should probably be something like QUEUE_SNAPSHOT or something along those lines. length is one of the attributes and doesn't really represent the payload.
ajaycj
changed the title
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
Signed-off-by: Ajay Chinthalapalli Jayakumar <[email protected]>
pyproject.tomlpip installsucceeds with a clean virtualenvThis update introduces the ability to use custom hooks for emitting events in Django projects, providing a flexible mechanism for developers to handle application events outside of New Relic. By configuring a custom hook, you can process events in a way that fits your application's needs, whether it's logging, sending notifications, or integrating with other third-party services.