-
Notifications
You must be signed in to change notification settings - Fork 35
refactor: ensure that using previous replaced does not remove files and reload unless necessary #310
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @richm, your pull request is larger than the review limit of 150000 diff characters
1248c50 to
1c35ffb
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #310 +/- ##
===========================================
- Coverage 61.09% 42.55% -18.55%
===========================================
Files 2 3 +1
Lines 910 2277 +1367
===========================================
+ Hits 556 969 +413
- Misses 354 1308 +954
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1c35ffb to
1f2d624
Compare
| if HAS_FIREWALLD: | ||
| firewall.config.FIREWALLD_POLICIES | ||
|
|
||
| HAS_POLICIES = True |
Check notice
Code scanning / CodeQL
Unused global variable Note
|
|
||
| HAS_POLICIES = True | ||
| except AttributeError: | ||
| HAS_POLICIES = False |
Check notice
Code scanning / CodeQL
Unused global variable Note
| try: | ||
| from firewall.functions import check_mac | ||
|
|
||
| HAS_CHECK_MAC = True |
Check notice
Code scanning / CodeQL
Unused global variable Note
|
|
||
| HAS_CHECK_MAC = True | ||
| except ImportError: | ||
| HAS_CHECK_MAC = False |
Check notice
Code scanning / CodeQL
Unused global variable Note
7d5e884 to
81292e4
Compare
|
[citest] |
81292e4 to
bb240fa
Compare
|
[citest] |
bb240fa to
a697edc
Compare
8db7dc8 to
d4a3ad6
Compare
…nd reload unless necessary The old implemenation of `previous: replaced` would remove all files and reload the firewall every time, even if not necessary - it wasn't technically idempotent, although it would check the new files to see if they matched the old files, and report `changed: false` if nothing actually changed state. The new implementation uses an in-memory backend to apply the changes, then checks if anything changed, and then removes the files and reloads firewall only if something actually changed. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
d4a3ad6 to
6ff5467
Compare
|
[citest] |
|
[citest_bad] |
The old implemenation of
previous: replacedwould remove all files and reload thefirewall every time, even if not necessary - it wasn't technically idempotent, although
it would check the new files to see if they matched the old files, and report
changed: falseif nothing actually changed state.The new implementation uses an in-memory backend to apply the changes, then checks if
anything changed, and then removes the files and reloads firewall only if something
actually changed.
Signed-off-by: Rich Megginson rmeggins@redhat.com