This is an automatically generated e-mail. To reply, visit: https://fedorahosted.org/reviewboard/r/307/

src/rolekit/server/dbusrole.py (Diff revision 1)
49
#    persistent = True
50
    """ Make RolekitD persistent. """
I suspect these two commented-out lines should be removed.

src/rolekit/server/dbusrole.py (Diff revision 1)
66
        path = "%s/%s" % (ETC_ROLEKIT_ROLES, self._name)
67
        if os.path.exists(path) and os.path.isdir(path):
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 (Diff revision 1)
270
            raise RolekitError(NAME_CONFLICT, settings.filename)            
Extra whitespace at the end of this line

src/rolekit/server/roled.py (Diff revision 1)
228
    @dbus_service_method(DBUS_INTERFACE, in_signature='',
229
                         out_signature='ao')
230
    @dbus_handle_exceptions
231
    def getAllRoleInstances(self, sender=None):
232
        """ return the list of all role instances """
233
        log.debug1("getAllRoleInstancesByState()")
This debug message should list "getAllRoleInstances()", not "getAllRoleInstancesByState()"

- Stephen Gallagher


On July 10th, 2014, 12:37 p.m. EDT, Thomas Woerner wrote:

Review request for rolekit reviewers, Stephen Gallagher and Thomas Woerner.
By Thomas Woerner.

Updated July 10, 2014, 12:37 p.m.

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)

View Diff