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
Add cloudwatch Target #184
Conversation
12b5de2
to
0a6f647
Compare
When can we expect this feature to be merged. I am looking forward to use this feature for creating the AWS cloudwatch dashboards using grafanalib. |
0a6f647
to
b198528
Compare
a6543e2
to
938f976
Compare
@@ -7,7 +7,7 @@ | |||
envlist = py27, py35, py37 | |||
|
|||
[testenv] | |||
commands = pytest --junitxml=test-results/junit-{envname}.xml | |||
commands = pytest -o junit_family=xunit2 --junitxml=test-results/junit-{envname}.xml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating to new junit_family, related with:
pytest-dev/pytest#6179
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a separate PR maybe? It definitely makes sense:
.tox/py37/lib/python3.7/site-packages/_pytest/junitxml.py:436
/home/daniel/dev/grafanalib/.tox/py37/lib/python3.7/site-packages/_pytest/junitxml.py:436: PytestDeprecationWarning: The 'junit_family' default value will change to 'xunit2' in pytest 6.0.
Add 'junit_family=xunit1' to your pytest.ini file to keep the current format in future versions of pytest and silence this warning.
_issue_warning_captured(deprecated.JUNIT_XML_DEFAULT_FAMILY, config.hook, 2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it to #236
"AWS/WorkSpaces", | ||
] | ||
|
||
validRegions = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to use something like https://stackoverflow.com/a/38451512 here?
Just a quick PSA from me:
We'd love to see you there. |
|
||
|
||
dashboard = Dashboard( | ||
title="Clowdwatch Stats", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo here but it's inconsequential
@uritau - Please review the circlci tests, it looks like you'll need to make some formatting changes to pass the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check CircleCI
if self.checkParams: | ||
self.__checkParameters() | ||
|
||
return { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dict.update
returns None
(https://docs.python.org/3/library/stdtypes.html#dict.update) which means that the target json ends up being null
.
Thanks again for your PR. It's been a while since we reviewed this. We'll close the PR for now as grafanalib mainline has already moved on a bit. Please reopen the PR if you (or anyone else) wants to move this work forward again. We're also happy to chat with you on Slack if you have any questions. |
What does this do?
This PR adds a Cloudwatch Target to simplify the usage of Cloudwatch datasource.
.
Why is it a good idea?
This allows users to use Cloudwatch datasource with specific target (limiting and simplifiying the call) and doesn't interferes with anything else on grafanalib.
Questions
validNamespaces
andvalidRegions
is not the best approach, but I haven't been able to find any AWS API to gather this information without registering in AWS :( so that's why exists thecheckParams
, with false this will not be updated.Usage example