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.

Sign up for GitHub

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

Jump to bottom

[core] use internal /run for pid and pickle files #1569

Merged
merged 3 commits into from
May 12, 2015
Merged

[core] use internal /run for pid and pickle files #1569

merged 3 commits into from
May 12, 2015

Conversation

degemer
Copy link
Member

@degemer degemer commented Apr 22, 2015

  • use /run for run files (supervisor, pid and pickle), move more functions to utils/
  • start dogstatd and dd-agent only when needed
  • remove legacy option clean

See commits descriptions for more details.

It should affect:

And of course https://github.com/DataDog/dd-agent-omnibus, to add a internal /run directory (Windows is not concerned)

@degemer degemer added this to the 5.4.0 milestone Apr 22, 2015
@degemer degemer mentioned this pull request Apr 23, 2015
Merged
@degemer
Copy link
Member Author

degemer commented Apr 23, 2015

This PR should also allow us to run supervisor as dd-agent, and thus fix #1348.

@degemer degemer force-pushed the quentin/pid-picle-path-update branch 2 times, most recently from 6025e90 to 6005c6c Compare April 30, 2015 19:33
@LeoCavaille LeoCavaille self-assigned this May 4, 2015
@LeoCavaille
Copy link
Member

Makes sense to me.
However, I'll maybe still allow Platform (and PidFile although less likely) to be imported from where they were before. Nothing says they're part of a private API, and people might use those in custom checks to switch behaviors on different OSs, you can leave a compat import, clearly state that it is deprecated and add a FIXME for 6.x.

@degemer degemer force-pushed the quentin/pid-picle-path-update branch from 6005c6c to d85b733 Compare May 4, 2015 22:14
@degemer
Copy link
Member Author

degemer commented May 4, 2015

Sure, I added them back. I wanted to deprecate them, but it's a pain since all Platform methods are class methods, so I just added a comment instead.

@degemer degemer force-pushed the quentin/pid-picle-path-update branch from d85b733 to 544fcd3 Compare May 7, 2015 14:16
@remh
Copy link
Contributor

remh commented May 11, 2015

@degemer can you rebase please ?

remh
remh reviewed May 11, 2015
View reviewed changes
agent.py
@@ -77,6 +77,7 @@ def _handle_sigusr1(self, signum, frame):
self._handle_sigterm(signum, frame)
self._do_restart()

@classmethod
def info(self, verbose=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny tiny nitpick but if you're switching to a class method, it should be

cls

instead of

self

@remh
Copy link
Contributor

remh commented May 11, 2015

Great work overall! Just a bunch of tiny nitpicks and it's good to merge.

@degemer degemer force-pushed the quentin/pid-picle-path-update branch from 544fcd3 to a87e6c2 Compare May 11, 2015 20:00
@degemer
Copy link
Member Author

degemer commented May 11, 2015

Thanks Rémi! Updated and rebased.

@degemer
[core] use internal /run for temp files
6cb37ec 
Added it to pidfile (instead of `/var/run/dd-agent`) and pickle
files, as it should fix all issues with `dd-agent info` reporting
incorrectly. It doesn't break anything, as it checks first to see if
the internal `/run` exists, and fallback to the current behaviour if
not. (ie `tempfile.gettempdir()`)

Also:
- move PidFile and Platform from util.py to their own files in `utils/`
- as a consequence, move `checks/utils.py` into `utils/tailfile.py`
@degemer
[core] only start collector/dogstatsd when needed
6757c35 
Before any call to `agent.py` or `dogstatsd.py` would actually create a
`Agent` or `Dogstatsd` instance, without doing anaything with it (and
diplaying a message about its pidfile).

This commit changes this behaviour and only create them when needed.
@degemer
[core] remove old option --clean
e25f5c8 
It's a legacy command which shouldn't be needed after #1435
@degemer degemer force-pushed the quentin/pid-picle-path-update branch from a87e6c2 to e25f5c8 Compare May 12, 2015 15:48
@degemer degemer mentioned this pull request May 12, 2015
Merged
remh added a commit that referenced this pull request May 12, 2015
@remh
Merge pull request #1569 from DataDog/quentin/pid-picle-path-update
a911a35 
[core] use internal `/run` for pid and pickle files
@remh remh merged commit a911a35 into master May 12, 2015
@degemer degemer deleted the quentin/pid-picle-path-update branch May 12, 2015 19:02
@yannmh yannmh mentioned this pull request Jun 4, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@LeoCavaille LeoCavaille

Labels
core resilience
Projects
None yet
Milestone
5.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@degemer @LeoCavaille @remh