[Bug 1293100] Review Request: tarantool - an in-memory database and Lua application server

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1293100



--- Comment #16 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> ---
%defattr(-,root,root,-) is unnecessary
[https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions].

Requires: systemd-units → systemd-units has been merged into systemd for a long
while

Requires: tarantool = %{version} → Requires: tarantool%{?_isa} =
%{version}-%{release}

Summary: Common files, admin tools and init scripts. →
Summary: Common files and admin tools for %{name}
(no dot)

Requires: tarantool → Requires: tarantool = %{version}-%{release}

%if 0%{?fedora} >= 21
%license %{_datadir}/doc/tarantool/LICENSE
%else
%doc %{_datadir}/doc/tarantool/LICENSE
%endif
→
%{!?_licensedir:%global license %doc}
%doc LICENSE
(see https://fedoraproject.org/wiki/EPEL:Packaging#The_.25license_tag)


OK, packaging is looking mostly OK. Now we get to the hard stuff :(

/usr/lib/tarantool/tarantool.init
Please no. All this script does is run 'lua /usr/bin/tarantool something'. Just
put this directly in the systemd unit file. Things will be much simpler and
more robust.

/usr/lib/systemd/system/tarantool.service:
# It's not recommended to modify this file in-place, because it will be
# overwritten during package upgrades. If you want to customize there're
# number of ways:

# Recommended way:
# 1) Use "/etc/sysconfig/tarantool" or "/etc/default/tarantool" -
# They're supported by our start-stop utility - tarantoolctl

# Usual way for RPM-based distros
# 2) Create a file "/etc/systemd/system/tarantool.service",
# containing
#   .include /usr/lib/systemd/system/tarantool.service
#   # Here're your changes
#
# For example, if you want to change CONF_DIR create
# "/etc/systemd/system/tarantool.service" containing:
#   .include /usr/lib/systemd/system/tarantool.service
#   [Service]
#   Environment=CONF_DIR=/etc/tarantool/instances.other
# This will override the settings appearing below

.include stanza as been deprecated for a long time in systemd. It has been
replaced by drop-in directories [1], but it actually looks like you want
something else: systemd templates (also described in [1]).

[1]
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#Description

Something roughly like this:
/usr/lib/systemd/system/tarantool@.service:

[Unit]
Description=Tarantool instance %I
After=network.target
Documentation=man:tarantool(1)

[Service]
Type=forking
User=tarantool

ExecStart=/usr/bin/lua /usr/bin/tarantool start %I
ExecStop=/usr/bin/lua /usr/bin/tarantool stop %I

TimeoutSec=300

(Is such a long timeout really needed?)


To summarize the init part:
- remove obsolete advice from the unit file
- do not install the useless SysVinit wrapper script
- one of two options:
  - update tarantool.service to run /usr/bin/tarantool start for each instance
in /etc/tarantool/instances.enabled

  (or preferrably)
  - replace tarantool.service with a bunch of tarantool@.service instances like
described above, and use systemd enable/disable to manage instance enablement.

The second option is better in the long run, because those instances are more
isolated, and can be managed using standard system tools. But in the short run
it's more work, and it changes the way instances are enabled.


What about msgpuck? It seems that you split it out for review in #1295217?
Shouldn't it be a dependency? And lua-fun?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]