Merge lp:~statik/ubuntu/karmic/couchdb/fix-bug427036 into lp:ubuntu/karmic/couchdb

Proposed by Elliot Murphy
Status: Rejected
Rejected by: James Westby
Proposed branch: lp:~statik/ubuntu/karmic/couchdb/fix-bug427036
Merge into: lp:ubuntu/karmic/couchdb
Diff against target: None lines
To merge this branch: bzr merge lp:~statik/ubuntu/karmic/couchdb/fix-bug427036
Reviewer Review Type Date Requested Status
Martin Pitt Abstain
Registry Administrators Pending
Review via email: mp+11543@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Elliot Murphy (statik) wrote :

This adds a new variable in /etc/default/couchdb to control whether couchdb is started at boot or not - most desktop installs will not need couchdb started at boot so this is a nice compromise. It seemed simpler that splitting the init script into a separate binary package, and a few other packages such as pulseaudio, monit, and fetchmail seem to do the same thing.

Revision history for this message
John Lenton (chipaca) wrote :

Also, the fix was pair-programmed between Elliot and myself, so its awesomeness is insurmountable.

Revision history for this message
James Westby (james-w) wrote :

> This adds a new variable in /etc/default/couchdb to control whether couchdb is
> started at boot or not - most desktop installs will not need couchdb started
> at boot so this is a nice compromise. It seemed simpler that splitting the
> init script into a separate binary package, and a few other packages such as
> pulseaudio, monit, and fetchmail seem to do the same thing.

Most packages that use this default to starting the daemon.

pulseaudio doesn't because Ubuntu's default set-up uses a per-user setup,
and fetchmail is often used in the same way.

Granted you are doing this because you use couchdb in a similar way, but
it's still a system db on it's own, and that's currently how most people
use it.

Therefore I am a little wary of this change, but not against it. 2 questions:

  1) I thought the plan was to have a couchdb-bin package to avoid exactly
     this sort of issue.
  2) Have you spoken to the Debian maintainer about this?

Thanks,

James

Revision history for this message
Elliot Murphy (statik) wrote :

Thanks for the quick review!

> Most packages that use this default to starting the daemon.
>
> pulseaudio doesn't because Ubuntu's default set-up uses a per-user setup,
> and fetchmail is often used in the same way.

Neither does fetchmail or monit, which are the other two packages we looked at two get an idea of existing common practice.

>
> Granted you are doing this because you use couchdb in a similar way, but
> it's still a system db on it's own, and that's currently how most people
> use it.
>
> Therefore I am a little wary of this change, but not against it. 2 questions:
>
> 1) I thought the plan was to have a couchdb-bin package to avoid exactly
> this sort of issue.

I've discussed creating a separate couchdb-server binary package that contains only the init script, yes. However I didn't find any example of another package that was structured this way, while John showed me several packages that were using the variable-in-config-file approach. Also, splitting out the separate couchdb-server binary package for the init script would require making changes in the packages that depend on couchdb, which current only seems to be chef-server.

So I'm not opposed to going back and redoing this fix by creating a separate binary package for the init script, but this seemed like a better and simpler way to fix the problem.

> 2) Have you spoken to the Debian maintainer about this?

The init scripts are actually shipped by upstream, and I've submitted the patch to couchdb: https://issues.apache.org/jira/browse/COUCHDB-499

Considering this, what approach do you want to take? If you are convinced that splitting the init script really is the preferable solution, I'm willing to go back and do the change that way.

Revision history for this message
James Westby (james-w) wrote :

> I've discussed creating a separate couchdb-server binary package that contains
> only the init script, yes. However I didn't find any example of another
> package that was structured this way, while John showed me several packages
> that were using the variable-in-config-file approach. Also, splitting out the
> separate couchdb-server binary package for the init script would require
> making changes in the packages that depend on couchdb, which current only
> seems to be chef-server.

Well, I would suggest that if we do this the "couchdb" package is the one
with the init script, and the couchdb-bin package contains the executable.
That way "apt-get install couchdb" does what an admin would expect and
desktopcouch can just depend on couchdb-bin if I understand correctly,
and so the init script won't end up in the default install and we don't
have to worry about this.

> So I'm not opposed to going back and redoing this fix by creating a separate
> binary package for the init script, but this seemed like a better and simpler
> way to fix the problem.

Yes, I agree, my concern is just for admins, who may expect "apt-get install
couchdb" to give them a couchdb, and in general installing a server package
gets you a running server on Debian/Ubuntu. As you found there are cases
where that isn't the case, but we should at least justify it.

> > 2) Have you spoken to the Debian maintainer about this?
>
> The init scripts are actually shipped by upstream, and I've submitted the
> patch to couchdb: https://issues.apache.org/jira/browse/COUCHDB-499

I was also wondering about the "default off" approach, if Debian disagrees then
this is a delta we have to carry forever. If they prefer the split package
approach say then it isn't.

> Considering this, what approach do you want to take? If you are convinced that
> splitting the init script really is the preferable solution, I'm willing to go
> back and do the change that way.

I don't know what the preferred solution would be, I wanted to have
a discussion to see if we could decide that.

Thanks,

James

Revision history for this message
Martin Pitt (pitti) wrote :

> 1) I thought the plan was to have a couchdb-bin package to avoid exactly
> this sort of issue.

Right, I thought that was the plan, too.

> 2) Have you spoken to the Debian maintainer about this?

This is the crucial point here. There are other packages like chef-server or bindwood which depend on couchdb, and might rely on a system instance getting started. We should only change the behaviour of the couchdb package if (1) the reverse dependencies get along with this, i. e. don't expect a system instance, and (2) the Debian maintainer agrees with this change, so that we don't need to keep a packaging delta for all reverse dependencies just to cope with a different behaviour of couchdb.

If that means that the only thing in couchdb is the init script, so be it. There might be other things, though, like the /etc/couchdb/ hierarchy, /var/log/couchdb/, or /etc/logrotate.d/couchdb

Oh, and can we please stop the package from shipping /var/log/couchdb/0.10.0~svn809550 ? :-)

review: Abstain
Revision history for this message
Elliot Murphy (statik) wrote :

>
> If that means that the only thing in couchdb is the init script, so be it.
> There might be other things, though, like the /etc/couchdb/ hierarchy,
> /var/log/couchdb/, or /etc/logrotate.d/couchdb

OK, I'll rework the packages to move the init script. I don't think it would be appropriate to move the ini files or log directories from couchdb-bin; we don't want to disable couchdb from running at all we only want to stop it from running by default.

> Oh, and can we please stop the package from shipping
> /var/log/couchdb/0.10.0~svn809550 ? :-)

That appears to be a deliberate decision to avoid mixing logs from different snapshots of couchdb, and will need to be discussed with upstream. If you can file a separate bug with the reason why it should be removed I can certainly raise it with upstream.

Hmm, something must have changed. I don't see a way to set this merge proposal to rejected, only a way to delete it.

Unmerged revisions

16. By Elliot Murphy

* debian/patches/control-service-start.patch
  - Add a variable to /etc/default/couchdb to control whether the system
    couchdb starts up. (LP: #427036)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2009-08-31 09:06:26 +0000
3+++ debian/changelog 2009-09-10 16:54:18 +0000
4@@ -1,3 +1,12 @@
5+couchdb (0.10.0~svn809550-0ubuntu2) karmic; urgency=low
6+
7+ [John Lenton]
8+ * debian/patches/control-service-start.patch
9+ - Add a variable to /etc/default/couchdb to control whether the system
10+ couchdb starts up. (LP: #427036)
11+
12+ -- Elliot Murphy <elliot@ubuntu.com> Thu, 10 Sep 2009 12:33:00 -0400
13+
14 couchdb (0.10.0~svn809550-0ubuntu1) karmic; urgency=low
15
16 * New snapshot of couchdb 0.10.x stable prerelease branch (LP: #421971)
17
18=== added file 'debian/patches/control-service-start.patch'
19--- debian/patches/control-service-start.patch 1970-01-01 00:00:00 +0000
20+++ debian/patches/control-service-start.patch 2009-09-10 16:54:18 +0000
21@@ -0,0 +1,24 @@
22+diff -Nur -x '*.orig' -x '*~' couchdb/etc/default/couchdb couchdb.new/etc/default/couchdb
23+--- couchdb/etc/default/couchdb 2009-09-10 12:18:06.756299731 -0400
24++++ couchdb.new/etc/default/couchdb 2009-09-10 12:25:45.140341641 -0400
25+@@ -5,3 +5,5 @@
26+ COUCHDB_STDERR_FILE=/dev/null
27+ COUCHDB_RESPAWN_TIMEOUT=5
28+ COUCHDB_OPTIONS=
29++# Set COUCHDB_STARTUP=TRUE to make couchdb start at boot.
30++COUCHDB_STARTUP=
31+diff -Nur -x '*.orig' -x '*~' couchdb/etc/init/couchdb.tpl.in couchdb.new/etc/init/couchdb.tpl.in
32+--- couchdb/etc/init/couchdb.tpl.in 2009-09-10 12:18:06.756299731 -0400
33++++ couchdb.new/etc/init/couchdb.tpl.in 2009-09-10 12:31:48.332473763 -0400
34+@@ -41,6 +41,11 @@
35+ . $CONFIGURATION_FILE
36+ fi
37+
38++if [ ! "x$COUCHDB_STARTUP" = "xTRUE" -a ! "$1" = "stop" ]; then
39++ echo "Edit /etc/default/couchdb to start couchdb"
40++ exit 0
41++fi
42++
43+ log_daemon_msg () {
44+ # Dummy function to be replaced by LSB library.
45+

Subscribers

People subscribed via source and target branches

to all changes: