On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
src/rolekit/server/rolebase.py, lines 1263-1264 http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1220line1263
Why is `RoleDeploymentValues` derived from dict? The class doesn't implement any part of the dict interface, nor does any code seem to use the dict interface on any of its objects.
I was copying stuff around and forgot to drop that.
On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
src/rolekit/server/rolebase.py, line 1313 http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1220line1313
Why not simply attempt duck-typing? Should we care if units is a real list, rather than an object that is sequence-like enough to `extend()` and existing list?
I just dropped this assertion. I think it was a leftover from an earlier version of that function.
On Nov. 16, 2015, 3:40 p.m., Nils Philippsen wrote:
src/rolekit/server/rolebase.py, lines 1321-1336 http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/2/?file=1220#file1220line1321
How about: ``` # == Create the [Unit] Section == # unitfile['Unit'] = { 'Description': self._desc, 'Requires': tuple(self._requirements), 'BindsTo': tuple(self._requirements), # All roles are also assumed to require the network, so # we will start it after network.target 'After': "network.target", } # == Create the [Install] section == # unitfile['Install'] = { 'RequiredBy': tuple(self._requirements), # All roles are enabled for the multi-user.target 'WantedBy': "multi-user.target", } ```
Honestly, I find that considerably less readable than my version.
- Stephen
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/231/#review556 -----------------------------------------------------------
On Nov. 20, 2015, 8:51 p.m., Stephen Gallagher wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/231/
(Updated Nov. 20, 2015, 8:51 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen Gallagher, and Thomas Woerner.
Repository: rolekit
Description
We no longer require the failure unit or the extension units. Now that we are getting the state from systemd directly, we will simply set up the Requires/RequiredBy/BindsTo settings appropriately so that rolekit and systemd are always kept in sync.
Diffs
config/roles/databaseserver/role.py 1af90551aff0f681ee8c2f787780e76f24330927 config/roles/domaincontroller/role.py 9fefd60aed47888d2e7884a4b70a6ad76dd104d8 config/roles/memcache/role.py d12974128a75ef38666bbdd7d70b7623631e3166 config/roles/testrole/role.py b5fda8dc92b42735a7b0ce5c6b21fed7cf6fd015 src/rolekit/server/io/systemd.py b105483c7fa689a43a960ef78aa1105f7aa58b42 src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/231/diff/
Testing
Thanks,
Stephen Gallagher
rolekit-commits@lists.fedorahosted.org