feat(grafana_dashboard): Directly provide dashboard content #426
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
==========================================
+ Coverage 72.84% 75.53% +2.69%
==========================================
Files 21 17 -4
Lines 2261 2089 -172
Branches 475 457 -18
==========================================
- Hits 1647 1578 -69
+ Misses 445 340 -105
- Partials 169 171 +2
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:
|
|
Hi, it's great that you got started on this so quickly. We definitely need integration testing for this new feature. There are plenty of examples you can use as orientation. |
|
Of course. I first wanted to know if I was going in the right direction, since this is my first contribution.. |
* main: docs: added new fragments for release docs: added new fragments for release docs: update galaxy docs: update changelog test(grafana_dashboard): check if v11 subfolder api available chore(grafana_dashboard): search for folder by title and uid chore(grafana_dashboard): role parent_folder reference test(grafana_dashboard): refactor folder and subfolder tests chore(grafana_dashboard): rename parent_uid to parent_folder docs(grafana_dashboard): change text of changelog fragment Allow managing dashboards in subfolders
Hipska
force-pushed
the
feat/dashboard_content
branch
from
a5f6d29 to
7258964
Compare
|
@Nemental is this going in the right direction? I want to know so I can start making integration tests for it.. |
| dashboard_revision=dict(type="str", default="1"), | ||
| overwrite=dict(type="bool", default=False), | ||
| commit_message=dict(type="str"), | ||
| content=dict(type=dict_str), |
There was a problem hiding this comment.
https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#argument-spec
| content=dict(type=dict_str), | |
| content=dict(type="raw"), |
I think it's better, if not mandatory, to stick with the officially specified argument types. With raw, you can pass the content unvalidated and then perform the actual validation of the content immediately afterward.
There was a problem hiding this comment.
Okay, might need help on that. I copied the usage from here: https://github.com/ansible-collections/kubernetes.core/blob/94c1f57f36a08c1ac245556824410c52d929ec21/plugins/module_utils/args_common.py#L85
I think it is like that to be able to have ansible-lint to check the argument type?
There was a problem hiding this comment.
For example, you can set the type="raw" and then add your try-isinstance and except-TypeError to the already existing try/except block. Something like this (not tested):
try:
if not isinstance(module.params["content"], (dict, string_types)):
raise TypeError("content must be either a string or a dictionary")
if module.params["state"] == "present":
result = grafana_create_dashboard(module, module.params)
elif module.params["state"] == "absent":
result = grafana_delete_dashboard(module, module.params)
else:
result = grafana_export_dashboard(module, module.params)
except (TypeError, GrafanaAPIException, GrafanaMalformedJson) as e:
module.fail_json(failed=True, msg="error : %s" % to_native(e))
except GrafanaDeleteException as e:
module.fail_json(
failed=True, msg="error : Can't delete dashboard : %s" % to_native(e)
)
except GrafanaExportException as e:
module.fail_json(
failed=True, msg="error : Can't export dashboard : %s" % to_native(e)
)There was a problem hiding this comment.
@rndmh3ro Please advise, so we can have this landed..
|
|
||
|
|
||
| def dict_str(value): | ||
| if isinstance(value, (dict, string_types)): | ||
| return value | ||
| raise TypeError |
There was a problem hiding this comment.
Moving dict_str to module_utils is fine, but not absolutely necessary, since this function isn't used multiple times.
There was a problem hiding this comment.
I did it as maybe other parts could use it as well. OK to keep?
You're doing great, thank you for your contribution! I've left a few comments. |
# Conflicts: # plugins/module_utils/base.py # plugins/modules/grafana_dashboard.py
2 participants
SUMMARY
Add an option to directly pass the dashboard definition instead of via path. This reduces the need to first generate/place a JSON file on the filesystem.
Fixes #417
ISSUE TYPE
COMPONENT NAME
grafana_dashboard
ADDITIONAL INFORMATION