Fix Content-Length Mismatch while using json and ujson #1403
Conversation
The current implementation of `sanic` attempts to make use of `ujson` if it's available in the system and if not, it will default to the inbuilt `json` module provided by python. The current implementation of `ujson` does not provide a mechanism to provide a custom `seperators` parameter as part of the `dumps` method invocation and the default behavior of the module is to strip all the spaces around seperators such as `:` and `,`. This leads to an inconsistency in the response length when the response is generated using the `ujson` and in built `json` module provided by python. To maintain the consistency, this commit overrides the default behavior of the `dumps` method provided by the `json` module to add a `seperators` argument that will strip the white spaces around these character like the default behavior of `ujson` This addresses the issue referenced in sanic-org#1398 Signed-off-by: Harsha Narayana <harsha2k4@gmail.com>
|
Awesome catch! Taking a look now. |
|
@ahopkins Adding a few more unit test with responses containing different data types. Please wait before the merge. |
Signed-off-by: Harsha Narayana <harsha2k4@gmail.com>
|
@ahopkins Done. |
|
No worries. I will not merge. As a general practice, we've adopted a strategy to have two release managers for a year cycle. Their job is to do all merges to master and steer the release cycle. Right now it is @sjsadowski and @yunstanford. But the implementation looks sound. And more tests can never hurt. |
Codecov Report
@@ Coverage Diff @@
## master #1403 +/- ##
==========================================
+ Coverage 81.3% 81.33% +0.02%
==========================================
Files 17 17
Lines 1696 1698 +2
Branches 322 322
==========================================
+ Hits 1379 1381 +2
- Misses 248 249 +1
+ Partials 69 68 -1
Continue to review full report at Codecov.
|
|
Looks like the CI jobs is stuck somewhere. It ran the Unit Tests for the second commit as well , but the results aren't being reported. Strange. |
|
checking Travis |
|
I've restarted travis ci build |
The current implementation of
sanicattempts to make use ofujsonif it's available in the system and if not, it will default to the inbuiltjsonmodule provided by python.The current implementation of
ujsondoes not provide a mechanism to provide a customseparatorsparameter as part of thedumpsmethod invocation and the default behavior of the module is to strip all the spaces around separators such as:and,. This leads to an inconsistency in the response length when the response is generated using theujsonand inbuiltjsonmodule provided by python.To maintain the consistency, this commit overrides the default behavior of the
dumpsmethod provided by thejsonmodule to add aseparatorsargument that will strip the white spaces around these character like the default behavior ofujsonThis addresses the issue referenced in #1398
PS: Fixed some basic linter reported issue in the test files as well.