Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release/6.10.0 #1757

Merged
merged 48 commits into from
Apr 4, 2024
Merged

Release/6.10.0 #1757

merged 48 commits into from
Apr 4, 2024

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
15 checks passed
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