[Serve] fix logging error on passing traceback object into exc_info #46105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Gene Su <e870252314@gmail.com>
GeneDer
added
bug
GeneDer
commented
Jun 18, 2024
| class MyDeployment: | ||
| def __call__(self) -> str: | ||
| return "Hello world!" | ||
| # Set log directory for the deployment. | ||
| @serve.deployment(LoggingConfig(logs_dir="/my_dir") | ||
| @serve.deployment(LoggingConfig(logs_dir="/my_dir")) |
There was a problem hiding this comment.
This is just a doc change, since I saw it while reading it. Unrelated to the actual bug.
edoakes
approved these changes
Jun 18, 2024
| @@ -100,7 +100,7 @@ def format(self, record: logging.LogRecord) -> str: | |||
| The formatted log record in json format. | |||
| """ | |||
| record_format = copy.deepcopy(self.component_log_fmt) | |||
| record_attributes = copy.deepcopy(record.__dict__) | |||
There was a problem hiding this comment.
no idea why this was here in the first place...
There was a problem hiding this comment.
I feel it's probably a copy pasta from the line above 😅
Signed-off-by: Gene Su <e870252314@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
The root cause for the logging error we saw was due to the client passing traceback object into
exc_infoduring logging. In serve logger we do a deepcopy on the log record's__dict__attribute which was unable to pickle the traceback for the deep copy. This PR removed the deepcopy completely since we do not modify to the copied dict at all. Also added a test to ensure the regression doesn't happen.Related issue number
Closes #45912
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.