Skip to content
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

Allow json.tool to have identical infile and outfile #78108

Open
michael-kuhlmann mannequin opened this issue Jun 21, 2018 · 11 comments
Open

Allow json.tool to have identical infile and outfile #78108

michael-kuhlmann mannequin opened this issue Jun 21, 2018 · 11 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@michael-kuhlmann
Copy link
Mannequin

michael-kuhlmann mannequin commented Jun 21, 2018

BPO 33927
Nosy @rhettinger, @ezio-melotti, @matrixise, @pablogsal, @remilapeyre, @michael-kuhlmann, @CharString
PRs
  • bpo-33927: Add support for same infile and outfile to json.tool #7865
  • bpo-33927: Pass stdin and stdout are default arguments to argpars infile/outfile #11992
  • bpo-33927: Handle pyton -m json.tool --json-lines infile infile #29478
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2018-06-21.10:45:40.446>
    labels = ['type-bug', 'library', '3.11']
    title = 'Allow json.tool to have identical infile and outfile'
    updated_at = <Date 2021-11-08.22:09:34.365>
    user = 'https://github.com/michael-kuhlmann'

    bugs.python.org fields:

    activity = <Date 2021-11-08.22:09:34.365>
    actor = 'CharString'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2018-06-21.10:45:40.446>
    creator = 'kuhlmann'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33927
    keywords = ['patch']
    message_count = 10.0
    messages = ['320151', '320152', '320173', '320302', '320326', '320328', '320329', '320368', '320729', '390853']
    nosy_count = 7.0
    nosy_names = ['rhettinger', 'ezio.melotti', 'matrixise', 'pablogsal', 'remi.lapeyre', 'kuhlmann', 'CharString']
    pr_nums = ['7865', '11992', '29478']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue33927'
    versions = ['Python 3.11']

    @michael-kuhlmann
    Copy link
    Mannequin Author

    michael-kuhlmann mannequin commented Jun 21, 2018

    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)

    @michael-kuhlmann michael-kuhlmann mannequin added the type-bug An unexpected behavior, bug, or error label Jun 21, 2018
    @matrixise
    Copy link
    Member

    yep, or you could use sponge

    cat example.json | python3 -m json.tool | sponge example.json

    a small workaround ;-)

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Jun 21, 2018

    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.

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Jun 23, 2018

    Hi, I proposed a path in #7865, I'm not sure if I can apply the label skip news or if only a reviewer can.

    @pablogsal
    Copy link
    Member

    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'>

    @pablogsal
    Copy link
    Member

    @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.

    @pablogsal
    Copy link
    Member

    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).

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Jun 24, 2018

    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?

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Jun 29, 2018

    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?

    @remilapeyre remilapeyre mannequin added stdlib Python modules in the Lib dir 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Jun 21, 2020
    @matrixise
    Copy link
    Member

    Hello @pablogsal,

    What do you think about the PR of Rémi?

    Thank you,

    @matrixise matrixise removed 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes labels Apr 12, 2021
    @matrixise matrixise added 3.11 only security fixes and removed 3.10 only security fixes labels May 5, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @furkanonder
    Copy link
    Contributor

    @pablogsal The issue seems to have been solved. Can we close the issue?

    cat example.json

    {"name": "John", "age": 30, "car": null}

    After python -m json.tool example.json example.json;

    cat example.json

    {
        "name": "John",
        "age": 30,
        "car": null
    }

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    Status: No status
    Development

    No branches or pull requests

    3 participants