-
Notifications
You must be signed in to change notification settings - Fork 12
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 graphyte to send metrics to graphite or carbon #95
Conversation
lib/vsc/utils/netcat.py
Outdated
""" | ||
# convert list to string if needed | ||
if isinstance(data, list): | ||
data = "\n".join(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.
this makes no sense. i would expect either some serialiser, or that it sends element per element
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.
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.
valid point on the list, I was only thinking about my use case. I'll remove that.
graphyte is not available on our systems and I don't think maintaining and rpm to replace 2 lines of code is a good idea.
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 quite sure the graphyte dev also thought that.
graphyte is 200 lines, and license allows us to include it in vsc-utils if you don't want to build a package for it.
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.
just to replace the following?
with socket.create_connection((host, port), timeout=timeout) as sock:
sock.sendall(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.
you are sure we will never need any of the features in graphyte? not so convinced tbh
@stdweird this now includes graphyte |
not that hard to do, but add some convenience so data can be string or list as well, and some basic logging.