-
Notifications
You must be signed in to change notification settings - Fork 10.8k
JsonItemExporter puts lone comma in the output if encoder fails #3090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Labels
Comments
Thanks @tlinhart - will take a look at making a fix to that right away. :) |
Hi, I am looking for a beginner issue to start working with. Any chances i can pick this up and start working on it? |
Marking as a good first issue as finishing #3111 may be easy based on the feedback provided there. |
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jun 15, 2023
- Add initial changes from cathal's PR - scrapy#3090
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jul 10, 2023
- Handle exception not to add empty item. - scrapy#3090
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jul 10, 2023
- Add comment for handling the exception - scrapy#3090
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jul 10, 2023
- Remove unused import - scrapy#3090
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jul 11, 2023
- Fix invalid json issue - scrapy#3090
adnan-awan
added a commit
to adnan-awan/scrapy
that referenced
this issue
Jul 12, 2023
- Perform CR changes - scrapy#3090
kmike
pushed a commit
that referenced
this issue
Jul 22, 2023
…5952) * Partial fix for #3090 - only addresses JSON feeds. * Adding test case for #3090 to Json Exporter * Changing the deliberate-fail JSON example to a complex * Further tightening JsonItemExporter behaviour to prevent corruption. 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. * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Add initial changes from cathal's PR - #3090 * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Handle exception not to add empty item. - #3090 * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Add comment for handling the exception - #3090 * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Remove unused import - #3090 * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Fix invalid json issue - #3090 * [scrapy] JsonItemExporter puts lone comma in the output if encoder fails - Perform CR changes - #3090 --------- Co-authored-by: Cathal Garvey <cathalgarvey@cathalgarvey.me>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If
JsonItemExporter
is unable to encode the item, it still writes a delimiter (comma) to the output file. Here is a sample spider:Encoding the second items fails:
The output looks like this:
This seems not to be a valid JSON file as e.g.
json.load()
andjq
fail to parse it.I think the problem is in
export_item
method ofJsonItemExporter
class where it outputs the comma before decoding the item. The correct approach would be to try to decode the item (possibly with other needed operations) and perform the write atomically.The text was updated successfully, but these errors were encountered: