-
Notifications
You must be signed in to change notification settings - Fork 682
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
[plugins/git/gitlab_statistics] Added plugin to display GitLab stats #1106
Conversation
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.
Thank you for this useful contribution!
Please take a look at my comments below.
Additionally you may want to move the plugin to plugins/gitlab
. Maybe other plugins will follow.
plugins/git/gitlab_statistics
Outdated
@@ -0,0 +1,96 @@ | |||
#!/usr/bin/env python3 | |||
# -*- python -*- | |||
# -*- coding: utf-8 -*- |
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.
If I remember correctly, this is a magic line only to be used for python2?
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.
I'm not sure, but I removed it now.
plugins/git/gitlab_statistics
Outdated
# -*- python -*- | ||
# -*- coding: utf-8 -*- | ||
# | ||
# Plugin to monitor Gitlab status |
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 use a perldoc header for the documentation of the plugin. This improves the visualization of the plugin gallery and for munin-get
. Look for =cut
in other plugins for examples, if you like.
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.
Done.
plugins/git/gitlab_statistics
Outdated
# Usage: Place in /etc/munin/plugins/ (or link it there using ln -s) | ||
# Add this to your /etc/munin/plugin-conf.d/munin-node: | ||
# [gitlab] | ||
# user nobody |
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.
You can omit user nobody
, since this is the default (at least for Debian).
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.
Done.
plugins/git/gitlab_statistics
Outdated
# Add this to your /etc/munin/plugin-conf.d/munin-node: | ||
# [gitlab] | ||
# user nobody | ||
# logarithmic 1 |
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.
I think, the following three lines should be prefixed with env.
?
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.
Indeed, oops.
plugins/git/gitlab_statistics
Outdated
|
||
|
||
def weakbool(x): | ||
u = x.upper().strip() |
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.
How about something as short as the following?
return x.lower().strip() in {"true", "yes", "y", "1"}
It is pity, that such function is not available in python's standard library.
(only distutils.util.strtobool
gets close to this)
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.
Done.
plugins/git/gitlab_statistics
Outdated
|
||
|
||
def autoconf(): | ||
print("no" if url is None else "yes") |
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 emit a reason for no
- e.g. no (missing 'hostname' and/or 'token' environment variable)
.
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.
Done (separated out the reasons, in fact).
plugins/git/gitlab_statistics
Outdated
print("""\ | ||
graph_title GitLab statistics | ||
graph_vlabel amount | ||
""" + ("graph_args --logarithmic" if logarithmic else "") + """ |
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.
Maybe a format string would be easier to read here? Or simply print individual lines.
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.
Changed it to a print for the first three lines, then a single conditional print for the last one.
plugins/git/gitlab_statistics
Outdated
""") | ||
for x in reqjson().keys(): | ||
print(x + ".label " + x) | ||
print(x + ".info " + x) |
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.
Feel free to omit type
and draw
, since these represent munin's defaults.
Additionally you may omit info
- or instead add a detailed explanation of each field.
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.
Done. (Gitlab docs themselves don't have any extra info on those variables, so I just removed the info.)
plugins/git/gitlab_statistics
Outdated
#%# capabilities=autoconf | ||
|
||
import os | ||
import requests |
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.
requests
is not part of the standard library. Thus you would need the following details to use it properly:
- use a conditional import
autoconf
responds withno (...)
if it is missing- add the requirement to the documentation header
Personally I use requests
only for interactive http retrieval (since it is easy to use), but I would refrain from using it in code due to its non-deterministic behaviour (e.g. #4842).
Instead you may want to use urllib
and json
(core libraries):
raw_data = urllib.request.urlopen(url)
content = json.loads(raw_data)
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.
Done.
plugins/git/gitlab_statistics
Outdated
graph_title GitLab statistics | ||
graph_vlabel amount | ||
""" + ("graph_args --logarithmic" if logarithmic else "") + """ | ||
graph_category gitlab\ |
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.
It is recommend to use one of the well-known categories - here devel
would be suitable.
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.
Changed that.
f8fdb92
to
4b80c73
Compare
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 take a look at my remaining hints.
btw: thank you for your recent flow of plugin improvements and new plugins of fine quality!
plugins/git/gitlab_statistics
Outdated
print("no ('hostname' envvar not set)") | ||
if 'token' not in os.environ: | ||
print("no ('token' envvar not set)") | ||
print("yes") |
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.
I think, you forgot the else
before. In general: only one line should be printed, thus a chain of elif/else
would be good.
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.
Done.
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.
I think, you still omitted the else
before the final print("yes")
:)
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.
Sorry - I forgot to "unresolve" this thread before :(
a9b1c79
to
5707e88
Compare
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.
Almost done! :)
plugins/git/gitlab_statistics
Outdated
print("no ('hostname' envvar not set)") | ||
if 'token' not in os.environ: | ||
print("no ('token' envvar not set)") | ||
print("yes") |
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.
I think, you still omitted the else
before the final print("yes")
:)
Graphs common GitLab statistics (user, repo, fork, ... count) using its HTTP API statistics endpoint (requires a token to be set and passed to the plugin)
Thank you! |
Graphs common GitLab statistics (user, repo, fork, ... count) using its
HTTP API statistics endpoint (requires a token to be set and passed to
the plugin)