Skip to content

Release/6.10.0#1757

Merged
ifrit98 merged 48 commits intomasterfrom
release/6.10.0
Apr 4, 2024
Merged

Release/6.10.0#1757
ifrit98 merged 48 commits intomasterfrom
release/6.10.0

Conversation

@ifrit98
Copy link
Contributor

@ifrit98 ifrit98 commented Mar 25, 2024

What's Changed

brueningf and others added 30 commits February 28, 2024 13:41
…-gracefully/phil

handle req args by parsing and raising
…ic-imports

Replace wildcard imports with specific imports
To enable and manage third party loggers, loguru needed to be replaced with the builtin logging library. To maintain functional parity and best manage the logging infra state, the new log manager was built using a state machine, in order to explicitly handle transitions between the various logger states.
Removed some commented code that will be replaced anyway.
In order to capture logs from processes, a QueueHandler must be added to the logger that is instantiated with the LoggingMachine queue. The file log, if enabled, uses the trace format output for all loggers regardless of higher level settings. This is to avoid modifying the formatter during program execution.
To enable stdout streaming of external process loggers within the main process, all log record handling has been moved to a single QueueListener owned by the LoggingMachine. This way, handlers can be added or removed as necessary. To ensure that state transitions are respected temporally, as the QueueListener operates in a separate thread, a Lock was added to control operations on anything related to the QueueListener.
Since underlying calls to the logger for the various log functions are also concurrent, and the QueueListener is running on a separate thread, these actions must be synchronized so that the order of operations for state setting and logging on either side are respected.
…tions.

Turns out, the only threaded component was the QueueListener, and instead of synchronizing the logger calls, the listener can simply be stopped during a state transition. Thankfully, the StateMachine API makes this change trivial.
Some house keeping to the LoggingMachine initialization, and added unit tests to ensure state transitions work as expected.
Copy link
Contributor

@sepehr-opentensor sepehr-opentensor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@ifrit98 ifrit98 merged commit c5dabe0 into master Apr 4, 2024
@thewhaleking thewhaleking deleted the release/6.10.0 branch October 2, 2024 14:53
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

Successfully merging this pull request may close these issues.

7 participants