----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/ -----------------------------------------------------------
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
- currently limited to one instance per role, see MAX_INSTANCES in config/ - new class _DEFAULTS in RoleBase - New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals - roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance) - Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs -----
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py 3d5a85e5deb186df2a13b41d6ac1285f88c89126 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am ee4e308af0e28243c83a9ee117dba3a55229e9ab
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/#review484 -----------------------------------------------------------
src/rolekit/server/dbusrole.py https://fedorahosted.org/reviewboard/r/307/#comment1021
I suspect these two commented-out lines should be removed.
src/rolekit/server/dbusrole.py https://fedorahosted.org/reviewboard/r/307/#comment1022
This is more of a question than a problem with the code:
Do we expect that role plugins will create this directory at installation time, or should we have rolekit create it if it does not already exist?
I noted when I tried to deploy() testrole that I needed to create this directory on the filesystem first.
If the expectation is that the roles should install this path, then we should probably update the spec file to do so for testrole (while we are developing).
src/rolekit/server/dbusrole.py https://fedorahosted.org/reviewboard/r/307/#comment1020
Extra whitespace at the end of this line
src/rolekit/server/roled.py https://fedorahosted.org/reviewboard/r/307/#comment1019
This debug message should list "getAllRoleInstances()", not "getAllRoleInstancesByState()"
- Stephen Gallagher
On July 10, 2014, 12:37 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/
(Updated July 10, 2014, 12:37 p.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
- currently limited to one instance per role, see MAX_INSTANCES in config/
- new class _DEFAULTS in RoleBase
- New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals
- roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance)
- Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py 3d5a85e5deb186df2a13b41d6ac1285f88c89126 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am ee4e308af0e28243c83a9ee117dba3a55229e9ab
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing
Thanks,
Thomas Woerner
On July 10, 2014, 9:49 p.m., Stephen Gallagher wrote:
src/rolekit/server/dbusrole.py, lines 66-67 https://fedorahosted.org/reviewboard/r/307/diff/1/?file=5265#file5265line66
This is more of a question than a problem with the code: Do we expect that role plugins will create this directory at installation time, or should we have rolekit create it if it does not already exist? I noted when I tried to deploy() testrole that I needed to create this directory on the filesystem first. If the expectation is that the roles should install this path, then we should probably update the spec file to do so for testrole (while we are developing).
Yes, the creation of ETC_ROLEKIT_ROLES should be done in the daemon in start if it is missing. Also a check for ROLEKIT_ROLES. I will submit another patch for this.
RoleSettings.write will create the role directory before writing out the role settings if it is missing in it's latest version.
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/#review484 -----------------------------------------------------------
On July 10, 2014, 6:37 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/
(Updated July 10, 2014, 6:37 p.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
- currently limited to one instance per role, see MAX_INSTANCES in config/
- new class _DEFAULTS in RoleBase
- New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals
- roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance)
- Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py 3d5a85e5deb186df2a13b41d6ac1285f88c89126 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am ee4e308af0e28243c83a9ee117dba3a55229e9ab
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/ -----------------------------------------------------------
(Updated July 11, 2014, 4:05 p.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description -------
- currently limited to one instance per role, see MAX_INSTANCES in config/ - new class _DEFAULTS in RoleBase - New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals - roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance) - Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs (updated) -----
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py acf58ff8222b8ffc68074dfac38635b815e5a1f7 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am 30b4a8ebccdaff4994c339925be4f97674a0d959
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing -------
Thanks,
Thomas Woerner
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/#review496 -----------------------------------------------------------
Ship it!
Ship It!
- Stephen Gallagher
On July 11, 2014, 10:05 a.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/
(Updated July 11, 2014, 10:05 a.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
- currently limited to one instance per role, see MAX_INSTANCES in config/
- new class _DEFAULTS in RoleBase
- New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals
- roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance)
- Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py acf58ff8222b8ffc68074dfac38635b815e5a1f7 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am 30b4a8ebccdaff4994c339925be4f97674a0d959
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing
Thanks,
Thomas Woerner
On July 11, 2014, 4:51 p.m., Stephen Gallagher wrote:
Ship It!
Pushed: https://git.fedorahosted.org/cgit/rolekit.git/commit/?id=df3350b49920ac760e1...
- Thomas
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/#review496 -----------------------------------------------------------
On July 11, 2014, 4:05 p.m., Thomas Woerner wrote:
This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/
(Updated July 11, 2014, 4:05 p.m.)
Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
Repository: rolekit
Description
- currently limited to one instance per role, see MAX_INSTANCES in config/
- new class _DEFAULTS in RoleBase
- New src/rolekit/server/dbusrole.py for presenting the role in D-Bus with DEFAULTS property, getInstances, getNamedInstance and deploy methods and InstanceAdded and InstanceRemoved signals
- roled: Use of new src/rolekit/server/dbusrole.DBusRole, new methods getAllRoleInstances and getAllRoleInstancesByState, removed getRolesByState (state only in an instance)
- Basic implementation of src/rolekit/server/rolebase.RoleBase deploy and decommission methods, new StateChanged signal
Diffs
src/rolekit/server/roled.py 9682988dbdf62e9331cb5b0d997abe0db417d9a6 src/rolekit/server/rolebase.py acf58ff8222b8ffc68074dfac38635b815e5a1f7 src/rolekit/server/dbusrole.py PRE-CREATION src/Makefile.am 30b4a8ebccdaff4994c339925be4f97674a0d959
Diff: https://fedorahosted.org/reviewboard/r/307/diff/
Testing
Thanks,
Thomas Woerner
rolekit-commits@lists.fedorahosted.org