On Aug. 20, 2014, 5:16 p.m., Thomas Woerner wrote:
> config/roles/domaincontroller/role.py, line 146
>
<
http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/2/?file=282#file282...
>
> Please use self._settings and not self._DEFAULTS here.
Thomas Woerner wrote:
If the deploy method will be called later on again (maybe in redeploy) then the
former settings will not be used.
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.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/63/#review240
-----------------------------------------------------------
On Aug. 20, 2014, 8:59 p.m., Stephen Gallagher wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-fedoraserver.rhcloud.com/r/63/
-----------------------------------------------------------
(Updated Aug. 20, 2014, 8:59 p.m.)
Review request for RoleKit Mailing List, Miloslav Trmac, Stephen Gallagher, Simo Sorce,
and Thomas Woerner.
Repository: rolekit
Description
-------
Domain Controller deployment
Diffs
-----
config/roles/domaincontroller/role.py 358deca3fc7172929d53d2c77efd5c919da2aea9
Diff:
http://reviewboard-fedoraserver.rhcloud.com/r/63/diff/
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).
Thanks,
Stephen Gallagher