Delete denied parameters from host interfaces #1356
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1356 +/- ##
==========================================
- Coverage 76.45% 73.66% -2.79%
==========================================
Files 43 35 -8
Lines 5445 4542 -903
Branches 1396 1210 -186
==========================================
- Hits 4163 3346 -817
+ Misses 814 788 -26
+ Partials 468 408 -60 ☔ View full report in Codecov by Sentry. |
plugins/modules/zabbix_host.py
Outdated
| for interface in exist_interfaces: | ||
| normalized_interface = interface | ||
| if LooseVersion("7.0") <= LooseVersion(zabbix_version): |
There was a problem hiding this comment.
Shouldn't this be >= ? 6.4 and 6.0 were working fine before.
There was a problem hiding this comment.
Is it wrong code? checking fields process will runs for 7.0 and higher.
There was a problem hiding this comment.
Right now you're checking for <= 7.0. That means it won't run in the future for 7.2, but it will run today for 6.4. 6.4 is working fine (don't think it needs the fields, but the API handles them gracefully). Either it needs to be >= 7.0 or we just sanitize them for all versions (which I don't honestly think is necessarily wrong).
There was a problem hiding this comment.
Right now you're checking for <= 7.0
Does this mean integration test target is 7.0 and lower?
if LooseVersion("7.0") <= LooseVersion(zabbix_version):
If 6.4 and 6.0, zabbix_host module's behavior is not chenged from current behavior. in addition, this effects on 7.0, 7.2 and higher.
Either it needs to be >= 7.0 or we just sanitize them for all versions (which I don't honestly think is necessarily wrong).
If you want to check for all version, I will remove version checking condition.
|
I'd remove the check and also add a change fragment. From there I think we're good. |
masa-orca
force-pushed
the
fix_host
branch
2 times, most recently
from
bec2045 to
bc52cd8
Compare
|
Integration tests failed. however it is not depends my change and I create another pull request to fix it. |
SUMMARY
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
In my change, I copied allowed_fields from this commit which is Vanav mentioned. Therefore, it will course errorif the allowed_fields chenges.