On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> src/rolekit/server/rolebase.py, lines 130-132
>
<
http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1209#file1...
>
> I'm not sure I'm too happy with overloading the state this way. Just
from looking at the code, I think it should work but I still think it would be cleaner if
we kept the state as `SYSTEMD` so it's explicit that one needs to consult systemd
(whether by listening to signals or polling) to find out the state of the target. AIUI,
this state gets saved to the JSON file in `/etc/rolekit/roles`, and I'd like it better
if none of "must consult systemd" states are ever persisted there.
OK, so it turns out that there was actually only one case where it would get written out
to disk: in the deferred role processing. I've modified that code so that it will
always explicitly set the state to NASCENT before writing out the deferred settings JSON.
I then modified rolesettings.py to only set the SYSTEMD state if something other than
NASCENT or ERROR is present in the settings data.
I've examined the rest of the code and with the exception of deferred processing,
settings.write() is never called anywhere before change_state(), so SYSTEMD should never
appear in the file.
On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> src/rolekit/server/io/systemd.py, lines 63-69
>
<
http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1208#file1...
>
> That's pretty minimalistic. Are you sure you don't want to put this in a
later commit that adds a little code? ;)
This was leftover from a first implementation that I ultimately scrapped but forgot to
remove. New version of the patch corrects this.
On Nov. 16, 2015, 3:19 p.m., Nils Philippsen wrote:
> src/rolekit/server/io/rolesettings.py, line 30
>
<
http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/2/?file=1207#file1...
>
> This bogus import comes from review #226. Please remove it in that commit.
Done.
- Stephen
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/230/#review555
-----------------------------------------------------------
On Nov. 12, 2015, 9:21 p.m., Stephen Gallagher wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/230/
-----------------------------------------------------------
(Updated Nov. 12, 2015, 9:21 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Nils Philippsen, Stephen
Gallagher, and Thomas Woerner.
Repository: rolekit
Description
-------
Get role state from systemd
Diffs
-----
src/rolekit/config/__init__.py.in 05d9f7b2bda59cf0b4ed6e1f277880ff2092b3c7
src/rolekit/dbus_utils.py a5f5a1d5ee4f32865fca6459615ea3359f3ca11d
src/rolekit/server/io/rolesettings.py c2424f1b166eb9eec5c59e3b79047cb482ca9081
src/rolekit/server/io/systemd.py b105483c7fa689a43a960ef78aa1105f7aa58b42
src/rolekit/server/rolebase.py 510190a5fbd66858417e9f75f73ded63e6236b85
Diff:
http://reviewboard-fedoraserver.rhcloud.com/r/230/diff/
Testing
-------
Thanks,
Stephen Gallagher