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.

On August 21st, 2014, 3:03 p.m. UTC, Thomas Woerner wrote:

Drop my last comment..

We discussed this on IRC and decided that at least for now it makes sense to require subsequent deploy() attempts to submit the full set of values, not just the delta from the last attempt.


- Stephen


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