Support orjson as an alternative encoder/decoder#1383
Draft
sirosen wants to merge 2 commits intoglobus:mainfrom
Draft
Support orjson as an alternative encoder/decoder#1383sirosen wants to merge 2 commits intoglobus:mainfrom
orjson as an alternative encoder/decoder#1383sirosen wants to merge 2 commits intoglobus:mainfrom
Conversation
1dbcee8 to
76ff1fc
Compare
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.
Add support for user selection of `orjson` as a faster and more correct JSON encoder/decoder. Request encoding is done by sending encoded bytes through `requests` (which is supported), and request decoding is done via a helper which reads `requests.Response.content` (bytes). This is primarily applied in - the JSON request encoder - the Response class - the APIError base class Additionally, GlobusApp and Transfer retry policies decode response contents, and are here updated to load from bytes. `mypy` runs are expanded to include `orjson` as a dependency, and test runs include it via a factor. New tests can directly target the encoder class, but as there is no central point of control for decoding, there is no clearly testable element for that purpose. tox configuration and testing requirements are updated to pull in `orjson` when the `-orjson` factor is used. GitHub Actions testing is now expanded to include `-orjson` tests.
76ff1fc to
42e3999
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.
Rebased onto #1384 to solve tox/ci config
Add support for user selection of
orjsonas a faster and more correctJSON encoder/decoder. Request encoding is done by sending encoded bytes
through
requests(which is supported), and request decoding is donevia a helper which reads
requests.Response.content(bytes).This is primarily applied in
Additionally, GlobusApp and Transfer retry policies decode response
contents, and are here updated to load from bytes.
mypyruns are expanded to includeorjsonas a dependency, andtest runs include it via a factor.
New tests can directly target the encoder class, but as there is no
central point of control for decoding, there is no clearly testable
element for that purpose.
tox configuration and testing requirements are updated to pull in
orjsonwhen the-orjsonfactor is used.GitHub Actions testing is now expanded to include
-orjsontests.