pyinfra
pyinfra copied to clipboard
the start of an implementation of files.block
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:
- The current implementation uses
awk
which I assumed was a reasonably dependency as even busybody seems to have it ? - 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... - 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:
- Given a file is created in the case where it doesn't exist, should I allow the caller to supplier the
user
,group
andmode
? - Despite the temporary file being used, for consistency with other operations would a
backup
option be useful ? - I have some, admittedly not speedy, functional tests; is there somewhere they should be stored ?
Codecov Report
Merging #885 (962bb35) into 2.x (12e07d3) will increase coverage by
0.06%
. The diff coverage is98.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.
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 ?
Thanks @Fizzadar. I've opened #900 to track the need for assume_file_present
.