Replace KubernetesAnsibleModule class with dummy class #227
Conversation
There was a problem hiding this comment.
The structure of the change looks okay to me, though the change seems to have introduced a cascade of test failures. Assuming we resolve those I'm fine with this change.
I haven't taken a peek, but do you know if anything in the Kubevirt modules in community.general would blow up after making these changes; I noticed they import from KubernetesRawModule.
Yes, Since we removed KubernetesAnsibleModule from only one module, the rest are in pipeline.
Good. Thanks for reminding me that. I will take care of that as well once this is resolved. |
Codecov Report
@@ Coverage Diff @@
## main #227 +/- ##
==========================================
- Coverage 42.62% 42.50% -0.12%
==========================================
Files 4 4
Lines 610 614 +4
Branches 122 122
==========================================
+ Hits 260 261 +1
- Misses 303 306 +3
Partials 47 47
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
LGTM, but I would like to make sure at least one or two other maintainers deeper into any other module integrations (I know of kubevirt that might need adjustments, but I don't know if anyone else might be relying on KubernetesAnsibleModule...?).
The other question: would this be something we can merge into a 1.1, or do we need to go to 2.0.0 with this change?
|
@fabianvf could you weigh in on a dummy class here to avoid breaking anyone. |
| @@ -265,28 +274,6 @@ def diff_objects(existing, new): | |||
| result['after'] = diff[1] | |||
| return not diff, result | |||
|
|
|||
|
|
|||
| class KubernetesAnsibleModule(AnsibleModule, K8sAnsibleMixin): | |||
There was a problem hiding this comment.
We can probably leave this and the KubernetesRawModule classes around as dummies with warnings to avoid a breaking API change
* Make a AnsibleMixin parent class for every module * Remove KubernetesAnsibleModule class * Modified k8s_log Signed-off-by: Abhijeet Kasurde <[email protected]>
Signed-off-by: Abhijeet Kasurde <[email protected]>
Signed-off-by: Abhijeet Kasurde <[email protected]>
Signed-off-by: Abhijeet Kasurde <[email protected]>
Signed-off-by: Abhijeet Kasurde <[email protected]>
Signed-off-by: Abhijeet Kasurde <[email protected]>
* Add a dummy class for backward compatiblity Signed-off-by: Abhijeet Kasurde <[email protected]>
Akasurde
changed the title
|
I think this PR breaks the unit tests in community.general (see f.ex. https://app.shippable.com/github/ansible-collections/community.general/runs/4278/15/tests). Could someone please check whether this also affects the kubevirt modules? |
|
@felixfontein I have raised a PR to fix failing kubevirt tests. |
|
Community.general - ansible-collections/community.general#1070 |
SUMMARY
Signed-off-by: Abhijeet Kasurde [email protected]
ISSUE TYPE
COMPONENT NAME
plugins/module_utils/common.py
plugins/modules/k8s_log.py