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

[core] hard-deprecate start/stop/restart/status commands #3004

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

degemer
Copy link
Member

@degemer degemer commented Nov 7, 2016

What does this PR do?

This commits still allows these options, but only in developer mode,
removing the risk for most people to misuse them.

It also removes the deprecation notice for the old command line
executables: they are not shipped anymore, and the main reason why they
were deprecated was the start/stop/restart/status commands. Since this
commit takes care of that, the notices are not needed anymore.

Motivation

All of these are handled by Supervisord, which launches the processes in
the foreground. This is a complete legacy code, and some people still
use it wrongly.

@degemer degemer added this to the 5.11.0 milestone Nov 7, 2016
@degemer degemer force-pushed the quentin/remove-start-sto branch 2 times, most recently from ccd651d to 984aab6 Compare November 7, 2016 22:52
@gmmeyer gmmeyer self-assigned this Nov 7, 2016
All of these are handled by Supervisord, which launches the processes in
the foreground. This is a complete legacy code, and some people still
use it wrongly.
This commits still allows these options, but only in developer mode,
removing the risk for most people to misuse them.

It also removes the deprecation notice for the old command line
executables: they are not shipped anymore, and the main reason why they
were deprecated was the start/stop/restart/status commands. Since this
commit takes care of that, the notices are not needed anymore.
@degemer degemer changed the title [core] remove start/stop/restart commands [core] hide start/stop/restart/status commands Nov 8, 2016
@degemer degemer force-pushed the quentin/remove-start-sto branch from 984aab6 to 7580eba Compare November 8, 2016 16:48
@degemer degemer changed the title [core] hide start/stop/restart/status commands [core] hard-deprecate start/stop/restart/status commands Nov 8, 2016
Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

lgtm

@degemer degemer merged commit 9ade5c8 into master Nov 9, 2016
@degemer degemer deleted the quentin/remove-start-sto branch November 9, 2016 16:25
degemer added a commit that referenced this pull request Nov 15, 2016
* master: (254 commits)
  Reduce the maximum amount of argument to pylint.
  [forwarder] stop flushing after 10s
  [etcd] Report errors connecting to etcd endpoint (#3007)
  Postfix check should pass raise_on_empty_output=False
  [status] Silence requests exception
  [psutil] Only set `psutil.PROCFS_PATH` once in the collector (#3013)
  [ci] fix check name detection of test files (#3021)
  [ci][rabbitmq] Increase wait timeout (#3022)
  [core] SpooledTemporaryFile for subprocess output (#3002)
  [collector] isolate system checks (#3001)
  [ci] Fix Travis jobs timing out (#3017)
  [mongo] Use db.current_op instead of manually querying (#3016)
  [ci] fix bad citizens detection (#3020)
  [ci] add debug logs (#3019)
  [mongo] use `currentOp` for mongodb 3.2+
  Use proxy for API key check in info page (#3012)
  [mongo] Add MongoDB 3.2 support
  [mongo] Add MongoDB 3.2 to travis-ci config
  [packaging] 5.11.0 nightlies (#3009)
  [core] hard-deprecate start/stop/restart/status (#3004)
  ...
@masci masci modified the milestones: 5.11.0, 5.12.0 Jan 24, 2017
@miketheman
Copy link
Contributor

@degemer How are people meant to use non-supervisord approaches, where all we want is the dogstatsd listener and relay? (see linked issue above).

This has basically broken the Heroku Buildpack approach.

@degemer
Copy link
Member Author

degemer commented Mar 22, 2017

@miketheman Sorry to hear that. 😞
As a temporary workaround, you can set developer_mode: yes in datadog.conf - https://github.com/DataDog/dd-agent/blob/master/datadog.conf.example#L54. It will allow you to use the start command (and doesn't affect dogstatsd, developer_mode is only used in the collector).

I don't know Heroku, would it be possible to do the same as the dogstatsd-only docker image and run supervisor with a custom configuration (starting only dogstatsd & the forwarder)? https://github.com/DataDog/docker-dd-agent/blob/master/dogstatsd/Dockerfile & https://github.com/DataDog/docker-dd-agent/blob/master/dogstatsd/supervisor.conf
Or is it just impossible to run supervisor?

@irabinovitch
Copy link
Contributor

@jyee could you help take a look at this as well?

@miketheman
Copy link
Contributor

I'd point out that removing a working interface in a non-6.x version seems really wrong, SemVer-wise.

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

Successfully merging this pull request may close these issues.

5 participants