| msg320151 - (view) |
Author: Michael Kuhlmann (kuhlmann) |
Date: 2018-06-21 10:45 |
| It would be nice to have same infile and outfile for json.tool to replace json files with their pretty-printed version. Currently, if I try this I get an error: $ python3 -m json.tool example.json example.json Expecting value: line 1 column 1 (char 0) |
|
|
| msg320152 - (view) |
Author: Stéphane Wirtel (matrixise) *  |
Date: 2018-06-21 10:48 |
| yep, or you could use sponge cat example.json | python3 -m json.tool |
sponge example.json a small workaround ;-) |
|
| msg320173 - (view) |
Author: Rémi Lapeyre (remi.lapeyre) * |
Date: 2018-06-21 12:52 |
| Hi Michael, looking at the current code of json.tool, there is no reason for it not to be able to do this, I will a patch to do this tonight. |
|
|
| msg320302 - (view) |
Author: Rémi Lapeyre (remi.lapeyre) * |
Date: 2018-06-23 09:25 |
| Hi, I proposed a path in https://github.com/python/cpython/pull/7865, I'm not sure if I can apply the label `skip news` or if only a reviewer can. |
|
|
| msg320326 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2018-06-23 19:28 |
| The current status of json.tool also leaks a file descriptor if you use the same filename or an invalid one (needs debug build to receive this error message): $ ./python -m json.tool invalid_file.dat nofile.dat Expecting value: line 1 column 1 (char 0) sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='nofile.dat' mode='w' encoding='UTF-8'> $ ./python -m json.tool valid.dat valid.dat Expecting value: line 1 column 1 (char 0) sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='valid.dat' mode='w' encoding='UTF-8'> |
|
|
| msg320328 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2018-06-23 19:30 |
| @Rémi can you include a NEWS entry? Also, indicate that your patch prevents a file descriptor to be leaked in the cases indicated in my last message. |
|
|
| msg320329 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2018-06-23 19:41 |
| Maybe we should include a test that checks that if you provide an invalid file the file descriptors are not leaked (it can be in a different PR, maybe). |
|
|
| msg320368 - (view) |
Author: Rémi Lapeyre (remi.lapeyre) * |
Date: 2018-06-24 12:03 |
| Hi Pablo, while this patch should fix both problems, I'm not sure how to write a regression test for this, `assert_python_ok` in https://github.com/python/cpython/pull/7865/files#diff-7d4645839a05727ebdf39226fd56e29cR97 forks the interpreter so I'm not sure I can check for unclosed fd during the test. Should I call https://github.com/remilapeyre/cpython/blob/04b2ade751b318460c1f0f9566676ef519358328/Lib/json/tool.py#L18 directly and mock `io.open` and `os.close`? |
|
|
| msg320729 - (view) |
Author: Rémi Lapeyre (remi.lapeyre) * |
Date: 2018-06-29 18:45 |
| Hi Pablo, I added two tests to confirm that file descriptors do not link anymore. The tests are rather ugly but I'm not sure if it's possible to do better. Is this patch ok for you? |
|
|
| msg390853 - (view) |
Author: Stéphane Wirtel (matrixise) * |
Date: 2021-04-12 14:18 |
| Hello @pablogsal, What do you think about the PR of Rémi? Thank you, |
|
|