-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Fixed install/remove of influxdb on non-systemd Debian/Ubuntu systems #7933
Conversation
da65248
to
b23dcfc
Compare
b825676
to
d2a2096
Compare
For more rationale, please see similar PR on telegraf: influxdata/telegraf#2360
d2a2096
to
55106c1
Compare
@jwilder removed package.sh with latest rebase |
@jwilder i've moved that one to 1.3.0 as well. I really hope that you or some other maintainer is merging this soon as it is a bug and peeving not just us here at work 🤕 |
scripts/post-install.sh
Outdated
# Assuming SysVinit | ||
install_init | ||
# Run update-rc.d or fallback to chkconfig if not available | ||
if which update-rc.d >/dev/null; then |
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.
All calls to which
should be updated with &>/dev/null
, not >/dev/null
. On CentOS, I get this on a vanilla installation:
$ sudo rpm -i influxdb-1.2.1~rc3~cab47cd.x86_64.rpm
which: no update-rc.d in (/sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin)
which: no update-rc.d in (/sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin)
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.
fixed
scripts/post-install.sh
Outdated
# Amazon Linux logic | ||
install_init | ||
# Run update-rc.d or fallback to chkconfig if not available | ||
if which update-rc.d >/dev/null; then |
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.
Should be updated to &>/dev/null
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.
fixed. also there was a third place in this file which i also fixed ;)
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.
Needs minor changes to the which
calls. Other than that, this looks good.
I don't want to be crude but can anybody tell me please, why this hasn't been merged into 1.3.0? Is something missing or not working (i've tested this one extensively on several distros with either SysVinit and Systemd) |
According to last review changes in similar PR on influxdb: influxdata/influxdb#7933
According to last review changes in similar PR on influxdb: influxdata/influxdb#7933
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Why won't fix? If i repair the branch again, will it then be merged after more than 2 years? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically closed because it has not had recent activity. Please reopen if this issue is still important to you. Thank you for your contributions. |
Any updates on this? Can anyone of the reviewers @rossmcdonald @gunnaraasen say something about this? It's still an issue for us and not merged after 2 years. It's a shame. Please give some information wether it's planned to be merged or not. But please not just ignore it for a long time and let a bot close it. Thanks alot! |
See PR on telegrad for rationale: influxdata/telegraf#2360
Required for all non-trivial PRs