Refactor logging architecture #81
Conversation
airbrakes/data_handling/logger.py
Outdated
| @@ -158,61 +160,89 @@ def _log_the_buffer(self): | |||
| self._log_queue.put(buffered_message) | |||
| self._log_buffer.clear() | |||
|
|
|||
| def _logging_loop(self) -> None: | |||
| def _truncate_decimal_place(self, obj_type: Any) -> str: | |||
There was a problem hiding this comment.
idk if I love this name but that's a nit, if you think of anything better, change it
This feels like it could be in utils, but if it's never going to be used anywhere else, make it a static method
There was a problem hiding this comment.
I think methods which will only be used by the logger should not be in utils.
But yeah I'm changing the name
| return logged_data_packets | ||
|
|
||
| # ------------------------------- RUN IN A SEPARATE PROCESS ----------------------------------- |
There was a problem hiding this comment.
it helps newcomers realize which methods run in a separate process
There was a problem hiding this comment.
add this every other proceess then
| writer.writeheader() | ||
|
|
||
| # Makes a queue to store log messages, basically it's a process-safe list that you add to | ||
| # the back and pop from front, meaning that things will be logged in the order they were | ||
| # added. | ||
| # Signals (like stop) are sent as strings, but data is sent as dictionaries | ||
| self._log_queue: multiprocessing.Queue[dict[str, str] | str] = multiprocessing.Queue() | ||
| self._log_queue: multiprocessing.Queue[LoggedDataPacket | Literal["STOP"]] = ( |
There was a problem hiding this comment.
does it have to be "STOP"? Can this not be our stop signal constant?
There was a problem hiding this comment.
not in a Literal type. It has to be an Enum, or a actual string like that.
Changes:
Bug fix:
Performance:
Time to run test_integration.py:
On main: 19.42 seconds
This branch: 17.37 seconds
=> 10% faster