Fix JsonItemExporter puts lone comma in the output if encoder fails #5952
Conversation
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
Based on Mikhail's observation that to_bytes can fail also, leading to the same dangling comma as the failure to encode to JSON. Added a new test case to avoid reversion.
…ts_lone_comma-issue-3090
- Add initial changes from cathal's PR - scrapy#3090
|
Can you please check why some existing tests failed? Looks like some extra commas are added. |
- Handle exception not to add empty item. - scrapy#3090
- Add comment for handling the exception - scrapy#3090
- Remove unused import - scrapy#3090
Codecov Report
@@ Coverage Diff @@
## master #5952 +/- ##
==========================================
+ Coverage 88.86% 89.21% +0.34%
==========================================
Files 162 162
Lines 11199 11289 +90
Branches 1821 1833 +12
==========================================
+ Hits 9952 10071 +119
+ Misses 962 927 -35
- Partials 285 291 +6
|
- Fix invalid json issue - scrapy#3090
wRAR
reviewed
Jul 11, 2023
scrapy/exporters.py
Outdated
| itemdict = dict(self._get_serialized_fields(item)) | ||
| data = to_bytes(self.encoder.encode(itemdict), self.encoding) | ||
| except TypeError as e: | ||
| raise e |
There was a problem hiding this comment.
I don't think this catch+reraise does anything (except breaking the traceback IIRC)?
There was a problem hiding this comment.
But as this code does nothing then removing it won't break the test.
wRAR
reviewed
Jul 11, 2023
tests/test_exporters.py
Outdated
| self.assertRaises(TypeError, self.ie.export_item, i2) | ||
| self.ie.export_item(i3) | ||
| self.ie.finish_exporting() | ||
| print(f"values are=={self.output.getvalue()}") |
There was a problem hiding this comment.
oh yeah, I'll remove it thanks! :)
- Perform CR changes - scrapy#3090
wRAR
approved these changes
Jul 12, 2023
|
Thanks @adnan-awan! |
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.
Continuation of #3111.
Resolves #3090, closes #3111.