Add support for 'orjson' and introduce a 'ResponseDecoder' API for deserialization#1385
Draft
sirosen wants to merge 2 commits intoglobus:mainfrom
Draft
Add support for 'orjson' and introduce a 'ResponseDecoder' API for deserialization#1385sirosen wants to merge 2 commits intoglobus:mainfrom
sirosen wants to merge 2 commits intoglobus:mainfrom
Conversation
Previously, the test environments were binary flagged between `mindeps` and `!mindeps`. This is unfortunately bad for adding additional factors, as discovered when testing `orjson` as a factor. Because `tox` does not allow negative factors to be layered with "OR" semantics, it becomes difficult to deselect the locked requirements in `test.txt` in order to replace them. The base testenv is simplified to have unconditional dependencies, and factor-based dependency selection is done in descendant envs which can therefore fully replace the deps declared by the base env. The change in configuration also provides an opportunity to refactor the `depends` declaration into the list form which makes it inheritable.
f5f1b19 to
0a12f6a
Compare
In order to support `orjson` for serialization, add a new `OrjsonRequestEncoder`, which mirrors the `JSONRequestEncoder` type. The transport can select which encoder to use based on an init-time flag. This setting highlights the awkwardness of the class-level (read: global) mapping of strings to encoders. Therefore, it is now copied into an instance attribute, which is then post-hoc modified if `use_orjson=True`. On the decoding side, the story is more complex, as the objects doing decoding are various: responses, errors, and retry hooks. Only one of those three (retry hooks) operates at the abstraction layer of the transport. Therefore, the changes to support a defined decoder type, and provide it in these contexts, are as follows: - RetryContext objects now include the response decoder - response objects use their client object's decoder (specifically, `self.client.transport.decoder`) - API errors have their own decoder setting, which can be _injected_ via a contextvar API, but which is not publicly controllable via init -- this avoids compatibility issues around changing the init signature of our error types Selection of orjson is provided via an init arg to the transport and via an env var, `GLOBUS_SDK_USE_ORJSON=1`. If the setting is enabled but `orjson` is not installed, instantiating encoders and decoders (of the `Orjson*` types) will emit errors, to provide an early-error experience. Because API errors now need access to `globus_sdk.transport`, these import paths are more heavily implicated in other parts of the SDK. This breaks tests which help enforce deferred imports of `requests`, due to its very slow import time. To rectify, a number of `requests` imports, specifically, are now `TYPE_CHECKING` flagged and deferred. Tests are extended to cover `orjson` testing, including several new tests, an `orjson` dependency group, new frozen requirements, tox config, and CI config. Because responses read the client's decoder, a large number of test tweaks are needed to provide a "better" client mock which satisfies this requirement. In CI and the default tox env list, we only test `orjson` on a small selection of Python versions, to moderate the expansion of our test matrix.
0a12f6a to
ed61e11
Compare
This file contains hidden or 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
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.
Based on #1384 . This is an alternative to #1383.
In order to support
orjsonfor serialization, add a newOrjsonRequestEncoder, which mirrors theJSONRequestEncodertype.The transport can select which encoder to use based on an init-time flag.
This setting highlights the awkwardness of the class-level (read: global) mapping of strings to encoders. Therefore, it is now copied into an instance attribute, which is then post-hoc modified if
use_orjson=True.On the decoding side, the story is more complex, as the objects doing decoding are various: responses, errors, and retry hooks. Only one of those three (retry hooks) operates at the abstraction layer of the transport. Therefore, the changes to support a defined decoder type, and provide it in these contexts, are as follows:
RetryContext objects now include the response decoder
response objects use their client object's decoder (specifically,
self.client.transport.decoder)API errors have their own decoder setting, which can be injected via a contextvar API, but which is not publicly controllable via init -- this avoids compatibility issues around changing the init signature of our error types
Selection of orjson is provided via an init arg to the transport and via an env var,
GLOBUS_SDK_USE_ORJSON=1. If the setting is enabled butorjsonis not installed, instantiating encoders and decoders (of theOrjson*types) will emit errors, to provide an early-error experience.Because API errors now need access to
globus_sdk.transport, these import paths are more heavily implicated in other parts of the SDK.This breaks tests which help enforce deferred imports of
requests, due to its very slow import time. To rectify, a number ofrequestsimports, specifically, are nowTYPE_CHECKINGflagged and deferred.Tests are extended to cover
orjsontesting, including several new tests, anorjsondependency group, new frozen requirements, tox config, and CI config. Because responses read the client's decoder, a large number of test tweaks are needed to provide a "better" client mock which satisfies this requirement.In CI and the default tox env list, we only test
orjsonon a small selection of Python versions, to moderate the expansion of our test matrix.Diff Info
GitHub's diff looks massive because it's counting all those lines of lockfile/frozen-deps content. True diff is much more modest, but still nontrivial:
Changelog
Added
The SDK now supports use of
orjsonas an alternative JSON encoder and decoder.When
GLOBUS_SDK_USE_ORJSON=1is set, request sending and response decoding will useorjson.Use of
orjsonis optional, but if the variable is set andorjsonis not installed, errors will be emitted.The setting can also be configured on transport objects with the init option,
use_orjson=True.In a future major version of the SDK, use of
orjsonwill default to true when it is available.