Remove JSONEncoder monkeypatching
#14709
Conversation
conda/auxlib/entity.py
Outdated
| elif isinstance(obj, Path): | ||
| return str(obj) | ||
| return JSONEncoder.default(self, obj) | ||
| deprecated.constant("25.9", "26.3", "EntityEncoder", CondaJSONEncoder, addendum="Use `conda.common.serialize.json.CondaJSONEncoder` instead.",) |
There was a problem hiding this comment.
The title of this PR was misleading. This is not monkeypatching since we are not setting properties on the global json module. It is subclassing and using a standard json module feature.
This does seem like a slow function. Would a faster version keep a mapping of type(obj) -> (needs custom handling) to avoid doing hasattr all the time.
Or we continue to move away from auxlib and don't do any of this.
CodSpeed Instrumentation Performance ReportMerging #14709 will not alter performanceComparing Summary
|
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
CodSpeed WallTime Performance ReportMerging #14709 will not alter performanceComparing Summary
|
Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
| @@ -239,7 +239,6 @@ | |||
| from datetime import datetime | |||
| from enum import Enum | |||
| from functools import reduce | |||
| from json import JSONEncoder, dumps as json_dumps, loads as json_loads | |||
There was a problem hiding this comment.
Is this a Python optimization to try to avoid repeated "json.loads" lookups?
There was a problem hiding this comment.
I'm not sure, that seems unreasonable, the json module seems pretty straightforward about access to the various module level functions like loads
| @@ -15,16 +14,14 @@ | |||
| from traceback import format_exception, format_exception_only | |||
| from typing import TYPE_CHECKING | |||
|
|
|||
| from requests.exceptions import JSONDecodeError | |||
There was a problem hiding this comment.
and others...
Maybe the issue of requiring requests' JSONDecodeError is no longer a problem? In case requests is using simplejson apparently.
There was a problem hiding this comment.
Oh, that's a good point, at least the compat code still exists in requests, https://github.com/psf/requests/blob/91a3eabd3dcc4d7f36dd8249e4777a90ef9b4305/src/requests/exceptions.py#L31-L52
There was a problem hiding this comment.
Oh, I see our new json module here imports the correct compat exception class though: https://github.com/conda/conda/pull/14709/files#diff-84797ed824a58ab550574945be3861f96016e6b0fd2daa654b73925f398b77e2R11-R49
There was a problem hiding this comment.
Done something about it in 932b044 (#14709)
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Description
While investigating the recent build issues I noticed that we are importing and doing too much in
conda/__init__.py, especially with monkey patching the Python stdlib (something we should be moving away from whenever possible).So here we consolidate the json encoding logic into one place (
conda.common.serialize.json). We can mirror this move for the yaml logic in a subsequent PR.Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?