Skip to content

bpo-45644: json.tool: Use staged write for outfile.#30659

Closed
methane wants to merge 3 commits into
python:mainfrom
methane:json-infile-outfile
Closed

bpo-45644: json.tool: Use staged write for outfile.#30659
methane wants to merge 3 commits into
python:mainfrom
methane:json-infile-outfile

Conversation

@methane

@methane methane commented Jan 18, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

To support python -m json.tool --json-lines infile infile.

https://bugs.python.org/issue45644

@methane
bpo-45644: json.tool: Use staged write for outfile.
bac9e87 
Support python -m json.tool --json-lines infile infile
@methane
Add news.
30aed80
@CharString

CharString commented Jan 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

I don't particularly like it, it assumes write access to more than just inode of the original file. But 🤷 I won't me bikeshedding over this. The implementation reads well, the user facing interface is the same and I expect in 99% of the cases, write access won't be a problem.

kumaraditya303
kumaraditya303 reviewed Jan 18, 2022
View reviewed changes
Comment thread Lib/json/tool.py Outdated
@methane @kumaraditya303
Update Lib/json/tool.py
cf4cc58 
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@methane

methane commented Jan 18, 2022

Copy link
Copy Markdown
Member Author

Windows CI is failing:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "D:\a\cpython\cpython\Lib\json\tool.py", line 104, in <module>
    main()
    ^^^^^^
  File "D:\a\cpython\cpython\Lib\json\tool.py", line 94, in main
    with out as outfile:
    ^^^^^^^^^^^^^^^^^^^^
  File "D:\a\cpython\cpython\Lib\contextlib.py", line 144, in __exit__
    next(self.gen)
    ^^^^^^^^^^^^^^
  File "D:\a\cpython\cpython\Lib\json\tool.py", line 35, in _staged_outfile
    os.replace(tempname, path)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
PermissionError: [WinError 5] Access is denied: 'D:\\a\\cpython\\cpython\\build\\test_python_1644\ufffd\\test_python_worker_5368\ufffd\\@test_5368_tmp\ufffd.1kimt_1s' -> '@test_5368_tmp\ufffd'

It is difficult to write the staged write idiom with few lines of code.

For example: https://github.com/mitsuhiko/python-atomicfile/blob/master/atomicfile.py

I close this PR and wait that Python has this feature in stdlib.

@methane methane closed this Jan 18, 2022
@methane
methane deleted the json-infile-outfile branch January 18, 2022 11:30
@CharString CharString mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@methane @CharString @kumaraditya303 @the-knights-who-say-ni @bedevere-bot