-
Notifications
You must be signed in to change notification settings - Fork 48
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
[FEED] Close similarity between modules #176
Comments
I don't think, that a similarity regarding the code justifies merging those three things together, because the actual items that are created, are of different kinds. |
Hi Lars,
I agree that it does not simplify Ansible part.
It's definitely a good idea, because there are several functions that repeat cross the modules not only along the groups. What one can do for the moment, one can also add service groups module together with contact_groups or as a separate pull request. Best, |
@msekania please go for a dedicated PR this time, so we have a clean process. |
That's the approach I prefer across my other tools I am writing for Checkmk. Could be the right place to handle all API return codes and ansible messages (failed/changed/ok), too. I liked the http_code_mapping method when I first saw it, but did not invest the time to look everything up for my module. |
Is indeed a good feature! Definitely worth to adjust all the modules to it at some point. I also agree that messaging should go to utils.py, which will simplify maintenance of standard messages for all modules. Concerning the currently similar parts in contact, host, and service groups: The question remains though, should one unify internal subroutines (current complexity of REST API allows this) and if yes should one deposit them in utils.py, or in yet another dedicated file. |
Thanks for your input guys. I am glad we agree on this in general. If any of you feels like they have a good grip on this, feel free to propose a PR, and we can continue discussing there. |
This issue has been stale for 60 days. It will close in 7 days. |
This issue has been stale for 60 days. It will close in 7 days. |
I support the approach (thanks @msekania vor linking this issued to this PR) to make use of the module_utils library. In my PR I tried to make first step by implementing the API and a first example as a poc. Originally, I also had type_defs in this PR but eventually decided to keep the change as small as possible. I would like to make other proposals as soon as we agreed on a general approach. The structure would then be: |
My proposal for the structure:
An alternative could be to not make the utils over complex and just have a single file:
At the moment, I have no feeling about the need for utils for the other modules. On the other side: It's not a big move, to change from |
That sounds reasonable! So, let's start with a single utils.py file. |
This issue has been stale for 60 days. It will close in 7 days. |
Originally posted by @msekania in #168 (comment)
Porting Checklist
The text was updated successfully, but these errors were encountered: