----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs -----
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py b632321de63c331b3b3d0445d134fe1626e9944c src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
(Updated Čec. 30, 2014, 12:31 dop.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs (updated) -----
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py b632321de63c331b3b3d0445d134fe1626e9944c src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/#review187 -----------------------------------------------------------
src/rolekit/server/rolebase.py http://reviewboard-fedoraserver.rhcloud.com/r/54/#comment104
Not entirely happy with this, but at this point it still seems borderline readable enough, and adding a more descriptive API than just returning a dictionary would require a rethink of how the systemdJobHandler() object is used away from the current really-temporary-for-a-single-with-statement model. Ideas and fresh eyes welcome.
- Miloslav Trmac
On Čec. 30, 2014, 12:31 dop., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/
(Updated Čec. 30, 2014, 12:31 dop.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py b632321de63c331b3b3d0445d134fe1626e9944c src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
(Updated Čec. 30, 2014, 3:38 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs (updated) -----
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/#review225 -----------------------------------------------------------
TODO http://reviewboard-fedoraserver.rhcloud.com/r/54/#comment129
This portion no longer applies to the current master branch and needs to be rebased.
src/rolekit/dbus_utils.py http://reviewboard-fedoraserver.rhcloud.com/r/54/#comment130
We can probably write something up with unittest.mock, but let's not worry about that right now.
src/rolekit/server/rolebase.py http://reviewboard-fedoraserver.rhcloud.com/r/54/#comment131
This is almost exactly the same implementation as start_services_async. Could you merge these into a single function, please? Just pass a boolean value for starting and stopping.
The general approach looks great (and appears to work well). Just a few minor nitpicks to clean up.
- Stephen Gallagher
On July 30, 2014, 1:38 p.m., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/
(Updated July 30, 2014, 1:38 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
On Srp. 19, 2014, 2:41 odp., Stephen Gallagher wrote:
src/rolekit/server/rolebase.py, lines 438-450 http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/3/?file=237#file237line438
This is almost exactly the same implementation as start_services_async. Could you merge these into a single function, please? Just pass a boolean value for starting and stopping.
There are actually three moving parts (the logging, the manager call, and the error message), adding a boolean (or more parameters) would be adding ~7 lines to save 8 lines. Can we wait with this for restartServices? Then we will see what precisely are the common and unique parts, and see what the cleanest way to abstract things is. I don’t have a good intuition for this at the moment.
(This might involve s/SystemdJobHandler/SystemdServiceHandler/ and passing it the services, thus encapsulating the for loop, and a SystemdJobsResult object encapsulating the last three lines. But I’d rather see the last user first.)
- Miloslav
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/#review225 -----------------------------------------------------------
On Čec. 30, 2014, 3:38 odp., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/
(Updated Čec. 30, 2014, 3:38 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs
TODO 053560af7e811ae9d26b7eafff92ac2786af5408 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py d53c745352183944e307cca5259a58f058f81e27
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
(Updated Srp. 19, 2014, 4:53 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs (updated) -----
TODO 90fcb3f668aa382f0317092a25d794d37e012b23 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py a54a3b61b107775140a944a94bedd2dd0fa4ec81
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
(Updated Srp. 19, 2014, 9:27 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs (updated) -----
TODO 90fcb3f668aa382f0317092a25d794d37e012b23 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py a54a3b61b107775140a944a94bedd2dd0fa4ec81
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/#review233 -----------------------------------------------------------
Ship it!
Ship It!
- Stephen Gallagher
On Aug. 19, 2014, 7:27 p.m., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/
(Updated Aug. 19, 2014, 7:27 p.m.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs
TODO 90fcb3f668aa382f0317092a25d794d37e012b23 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py a54a3b61b107775140a944a94bedd2dd0fa4ec81
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
On Srp. 19, 2014, 9:36 odp., Stephen Gallagher wrote:
Ship It!
https://git.fedorahosted.org/cgit/rolekit.git/commit/?id=ac05751659b8272aa1c...
- Miloslav
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/#review233 -----------------------------------------------------------
On Srp. 19, 2014, 9:41 odp., Miloslav Trmac wrote:
This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/
(Updated Srp. 19, 2014, 9:41 odp.)
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs
TODO 90fcb3f668aa382f0317092a25d794d37e012b23 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py a54a3b61b107775140a944a94bedd2dd0fa4ec81
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/54/ -----------------------------------------------------------
(Updated Srp. 19, 2014, 9:41 odp.)
Status ------
This change has been marked as submitted.
Review request for RoleKit Mailing List, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well? At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.
Diffs -----
TODO 90fcb3f668aa382f0317092a25d794d37e012b23 config/roles/databaseserver/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9 src/rolekit/dbus_utils.py fb21cec2681f60ec2ad30372c4caf285e8ba13a7 src/rolekit/server/rolebase.py a54a3b61b107775140a944a94bedd2dd0fa4ec81
Diff: http://reviewboard-fedoraserver.rhcloud.com/r/54/diff/
Testing -------
starting/stopping postgres through d-feet.
Thanks,
Miloslav Trmac
rolekit-commits@lists.stg.fedorahosted.org