pyinfra icon indicating copy to clipboard operation
pyinfra copied to clipboard

the start of an implementation of files.block

Open morrison12 opened this issue 2 years ago • 1 comments

This is the beginning of an implementation of files.block (and files.Block) as contemplated in #565.

The main missing feature I can see is assume_file_present which is need for the common use case of installing a package and then configuring it although there could well be others.

Some "notes:

  1. The current implementation uses awk which I assumed was a reasonably dependency as even busybody seems to have it ?
  2. Not all awk implementations support in place editing so the code currently does the "write to a temporary file, if things worked then adjust the user/group/mode and mv it to the desired location" jig...
  3. I've tested this with a few awk implementations (both the native and GNU awks on macOS, GNU awk on Fedora and busybox awk in OpenWrt)

Some questions:

  1. Given a file is created in the case where it doesn't exist, should I allow the caller to supplier the user, group and mode ?
  2. Despite the temporary file being used, for consistency with other operations would a backup option be useful ?
  3. I have some, admittedly not speedy, functional tests; is there somewhere they should be stored ?

morrison12 avatar Sep 05 '22 22:09 morrison12

Codecov Report

Merging #885 (962bb35) into 2.x (12e07d3) will increase coverage by 0.06%. The diff coverage is 98.79%.

@@            Coverage Diff             @@
##              2.x     #885      +/-   ##
==========================================
+ Coverage   91.97%   92.03%   +0.06%     
==========================================
  Files         125      125              
  Lines        7837     7911      +74     
==========================================
+ Hits         7208     7281      +73     
- Misses        629      630       +1     
Impacted Files Coverage Δ
pyinfra/facts/files.py 99.27% <95.45%> (-0.73%) :arrow_down:
pyinfra/operations/files.py 98.76% <100.00%> (+0.11%) :arrow_up:
pyinfra/operations/util/files.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Sep 05 '22 23:09 codecov[bot]

The main missing feature I can see is assume_file_present which is need for the common use case of installing a package and then configuring it although there could well be others.

Ah yes, very unfortunate gotcha. This operation seems like it'd be particularly complex to support. Maybe it could just append the block, or maybe we don't support it? Happy to leave this out of scope for this PR entirely.

I'll happily file the bug report once this is released and work on a solution but need to find some time to make sure it covers all the cases. Perhaps mark the operation as "beta" in the meantime ?

morrison12 avatar Oct 01 '22 20:10 morrison12

Thanks @Fizzadar. I've opened #900 to track the need for assume_file_present.

morrison12 avatar Oct 24 '22 01:10 morrison12