Add aux_tag module #892
Add aux_tag module #892
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA or my organization already has a signed CLA. |
|
Hi @nicoske and thank you so much for your contribution! 🙏 |
lgetwan
left a comment
lgetwan
left a comment
There was a problem hiding this comment.
Thanks a lot for creating this module!
I added some comments, of which the one in the delete method might be the most crucial one.
| class AuxTagDeleteAPI(CheckmkAPI): | ||
| def delete(self): # Remove aux tag | ||
| return self._fetch( | ||
| code_mapping=HTTP_CODES_DELETE, | ||
| endpoint="/objects/aux_tag/%s" % self.params.get("name"), | ||
| method="DELETE", | ||
| ) | ||
|
|
||
|
|
||
| class AuxTagGetAPI(CheckmkAPI): | ||
| def get(self): | ||
| return self._fetch( | ||
| code_mapping=HTTP_CODES_GET, | ||
| endpoint="/objects/aux_tag/%s" % self.params.get("name"), | ||
| method="GET", | ||
| ) | ||
|
|
||
|
|
There was a problem hiding this comment.
Maybe I don't get the idea, but why have you created four ChecckmkAPI classes? I usually created only one, which has a put, get and delete method.
That way, you could also handle getting the etag and putting it into the header directly inside the class.
| auxtagupdate = AuxTagUpdateAPI(module) | ||
| auxtagupdate.headers["If-Match"] = result.etag | ||
| result = auxtagupdate.put() |
There was a problem hiding this comment.
Would be cool if you only update the aux tag if something has changed. (-> Idempotency).
| auxtagupdate.headers["If-Match"] = result.etag | ||
| result = auxtagupdate.put() | ||
|
|
||
| time.sleep(3) |
There was a problem hiding this comment.
Is the sleep needed to avoid race conditions?
However, I'd rather not do a sleep in the module.
| auxtagcreate = AuxTagCreateAPI(module) | ||
| result = auxtagcreate.post() | ||
|
|
||
| time.sleep(3) |
| auxtagdelete.headers["If-Match"] = result.etag | ||
| result = auxtagdelete.delete() | ||
|
|
||
| time.sleep(3) |
| return self._fetch( | ||
| code_mapping=HTTP_CODES_DELETE, | ||
| endpoint="/objects/aux_tag/%s" % self.params.get("name"), | ||
| method="DELETE", |
There was a problem hiding this comment.
This fails in my 2.4.0p14 environment, as deleting is done with a POST on the endpoint /objects/aux_tag/<aux_tag_name>/actions/delete/invoke.
3 participants
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Auxiliary tags can only be referenced in the tag_group module but cannot be managed.
What is the new behavior?
Other information
This module fills a gap to manage auxiliary tags creation and management before being referenced in tag groups.