Skip to content
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

Some enhacements #32

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Some enhacements #32

wants to merge 5 commits into from

Conversation

odivlad
Copy link

@odivlad odivlad commented Oct 15, 2015

Hey!

I've got some changes for your projects:

  1. Added Makefile with some rules (for example, make quickly builds a new rpm from scratch)
  2. New initscript check if daemon is already running or stopped (earlier you could run many processes at one moment)
  3. Removed rpm subpackages common, core (they're not nesessary at the moment, but some changes in files structure may be confused)
  4. Removed excess file gluster.py
  5. Changed default config (plugin_list enabled by default)

Hope, you'll like it. Please, feel free to comment my PR.

@TJM
Copy link
Collaborator

TJM commented Oct 15, 2015

Dang, now I need to try to merge this with https://github.com/LarkIT/newrelic-nfsiostat

@odivlad
Copy link
Author

odivlad commented Oct 15, 2015

@TJM, which way and what for?) I can help.

@TJM
Copy link
Collaborator

TJM commented Oct 15, 2015

@odivlad I forked this a while ago to make a NFS specific module, but the init script and stuff are painfully broken and LarkIT/newrelic-nfsiostat#20 - "python-daemon" is apparently EOL too, which is most of our init script issue

@odivlad
Copy link
Author

odivlad commented Oct 15, 2015

@TJM, don't you want to merge these two projects into one to avoid such merge problems? Here we can use logic with subpackages for different plugins, which can be optional for installation. May be, smth like this? As a package view odivlad@99050a7?
btw, python-daemon is not a problem at el6/el7.

EL6:

Name        : python-daemon
Arch        : noarch
Version     : 1.5.2
Release     : 1.el6
Size        : 88 k
Repo        : installed
From repo   : epel

EL7:

Available Packages
Name        : python-daemon
Arch        : noarch
Version     : 1.6
Release     : 4.el7
Size        : 26 k
Repo        : epel/x86_64

But I can try to write systemd-based daemonization.

@TJM
Copy link
Collaborator

TJM commented Oct 16, 2015

We split them because I wanted to focus on nfsiostat, and by splitting, I was able to control the "interface" in NewRelic. I basically included the source for nfsiostat (with some modifications to make it more library like) and otherwise wrapped the same code. I am sure there has been some divergence. It was not intended to be a "fork" that would be merged back in, but as they were a common codebase at one point, fixes on one sometimes apply to the other. :)

The biggest problem I see with mine on a day-to-day basis is that it doesn't "start" at boot time. It starts just fine if you run service newrelic-nfsiostat start but I noticed that problem for several of the newrelic plugins, not just this one.

@odivlad
Copy link
Author

odivlad commented Oct 18, 2015

@TJM, if you want, I can split this commit into "add make support" and "style fixes", so you'd be able to cherry-pick the first one with less pain.

@matteraf
Copy link

matteraf commented Jun 7, 2016

Traceback (most recent call last):
File "/usr/lib64/python2.6/logging/init.py", line 808, in emit
self.flush()
File "/usr/lib64/python2.6/logging/init.py", line 770, in flush
self.stream.flush()
IOError: [Errno 9] Bad file descriptor

@odivlad
Copy link
Author

odivlad commented Jun 8, 2016

@matteraf, how to reproduce?

@matteraf
Copy link

matteraf commented Jun 9, 2016

Just install it. Nothing more.

@odivlad
Copy link
Author

odivlad commented Jun 9, 2016

With which commands? My branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants