Hi Jan,
system-config/__init__:
44-46: Those definitions were initially there as a workaround to
circular import issues. Based on where they moved, and the addition of
more functions in-between imports at lines 44-100 (which I assume are
there due to additional ImportError issues found during development), I
think some slight re-factoring is needed.
I'm not sure what the best suggestion here is; I don't want to move
lines 44-100 into a new file called "sc_common" - I'm rather strongly
against that naming convention, when __init__ files serve that purpose
just fine. Perhaps moving everything after line 100 to a
solaris_install.sysconfig.cli module (and updating imports
appropriately) would be best. If this needs to be addressed in a
follow-up bug, then that would be acceptable (particularly since I
started it...).
network_info.py:
98-113: Please use solaris_install.Popen and condense this to something
like:
try:
dladm_popen = Popen(argslist, stdout=Popen.STORE,
stderr=Popen.STORE, logger=LOGGER())
except CalledProcessError as error:
LOGGER().warn("Error occurred during call to dladm: %s", error)
allowed_ips = dladm_popen.stdout.strip()
system-config/welcome.py:
73, 75, 77, 79, 81: These should be gettext'ed
Everything else looks great!
- Keith
On 05/ 3/11 10:21 AM, Jan Damborsky wrote:
Hi Keith,
could I please ask you to review changes for making interactive
configuration scenario zone aware ?
I would like to solicit comments from you, since significant amount
of these changes reside in SCI code and I want to make sure they
are compliant with the overall termui design.
Certainly comments from other people are more than welcome :-)
Webrev:
http://cr.opensolaris.org/~dambi/cr-7032012/
CR:
7032012 Make interactive system configuration scenario zone aware
Code review comments are to be accepted until COB Monday 5/9.
Touched Python files are believed to be pep8 clean :-)
Thank you very much,
Jan
Tests accomplished:
[1] Regression tests
* text install and AI ISO images built - LiveCD is to be
tested with final bits
* Text and Automated installations tested. LiveCD is to be tested
with final bits
[2] zones
* interactive configuration of global zone installed
from modified bits using Automated Installer
- it was verified that all configuration screens were
displayed. System was correctly configured.
* interactive configuration of non-global zone (NGZ) with
exlusive IP stack
- it was verified that following configuration screens were
displayed: hostname, networking, timezone, users
System was correctly configured.
* interactive configuration of non-global zone (NGZ) with
exlusive IP stack, 'allowed-address' property mandated to
one of NICs in that zone.
- it was verified that following configuration screens were
displayed: hostname, networking (NIC with 'allowed-address'
mandated from global zone was filtered out), timezone, users.
System was correctly configured.
* interactive configuration of non-global zone (NGZ) with
shared IP stack
- it was verified that following configuration screens were
displayed: hostname, timezone, users
System was correctly configured.
Known issues:
* Since DNS configuration is currently part of networking screens,
DNS can't be configured in non-global zone with shared IP stack.
That will be adjusted (if needed) after naming services screens
are separated as a result of enhancing SCI tool with support
for NIS and LDAP naming services (CR7031613).
* SCI tool does not support all permutations of configuration screens
(e.g. configuring just user/root account does not currently work).
In order to support that, SCI code will need to be refactored.
It will be addressed as separate issue, since it does not affect
supported zones scenarios.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
