This is an automatically generated e-mail. To reply, visit: http://reviewboard-fedoraserver.rhcloud.com/r/63/

On August 20th, 2014, 5:16 p.m. UTC, Thomas Woerner wrote:

config/roles/domaincontroller/role.py (Diff revision 2)
137
        props = copy.deepcopy(self._DEFAULTS)

Please use self._settings and not self._DEFAULTS here.

On August 20th, 2014, 5:24 p.m. UTC, Thomas Woerner wrote:

If the deploy method will be called later on again (maybe in redeploy) then the former settings will not be used.

On August 21st, 2014, 12:01 p.m. UTC, Thomas Woerner wrote:

Now self._settings and self._DEFAULTS is not used at all anymore. With this change there might be some settings missing for example in a (re)deploy call.

It might be good to call do_deploy in RoleBase with the a copy of _settings updated with values. RoleBase.deploy_async needs to be changed to apply this copy then after the do_deploy call to _settings again. RoleBase.apply_values would need to be changed to not complain on readonly settings but silently ignore them - which is not nice, but with the check_values call before do_deploy this should not lead into unexpected losses.

Drop my last comment..


- Thomas


On August 20th, 2014, 8:59 p.m. UTC, Stephen Gallagher wrote:

Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce, and Thomas Woerner.
By Stephen Gallagher.

Updated Aug. 20, 2014, 8:59 p.m.

Repository: rolekit

Description

Domain Controller deployment

Testing

Performed a mostly-successful deployment of FreeIPA onto a Fedora 21 VM.

(Mostly-successful because there appears to be an ipa-server-install bug preventing successful completion, but that should be irrelevant to this patch).

Diffs

  • config/roles/domaincontroller/role.py (358deca3fc7172929d53d2c77efd5c919da2aea9)

View Diff