Francesco Romani has posted comments on this change. Change subject: services, setup: ovirt-vmconsole integration ......................................................................
Patch Set 29: (10 comments) Thanks for the review! https://gerrit.ovirt.org/#/c/35906/29/packaging/setup/plugins/ovirt-engine-setup/vmconsole_proxy/config.py File packaging/setup/plugins/ovirt-engine-setup/vmconsole_proxy/config.py: Line 94: prompt=True, Line 95: default=True, Line 96: ) Line 97: Line 98: if enabled and not exists_user_group( > It's better to check VM Console Proxy availability before asking about conf Done Line 99: self.logger, Line 100: ovmpcons.PKIEnv.OVIRT_VMCONSOLE_ALLOWED_USER, Line 101: ovmpcons.PKIEnv.OVIRT_VMCONSOLE_ALLOWED_GROUP Line 102: ): Line 100: ovmpcons.PKIEnv.OVIRT_VMCONSOLE_ALLOWED_USER, Line 101: ovmpcons.PKIEnv.OVIRT_VMCONSOLE_ALLOWED_GROUP Line 102: ): Line 103: self.logger.warning( Line 104: 'VM Console Proxy seems not installed on this host. ' > Please use '_()' to allow localization Done Line 105: 'Disabled configuration.') Line 106: enabled = False Line 107: Line 108: self.environment[ Line 144: 'Engine vmconsole port [@DEFAULT@]: ' Line 145: ), Line 146: prompt=True, Line 147: default=self.environment[ Line 148: ovmpcons.ConfigEnv.VMCONSOLE_PROXY_PORT] > Please add a newline before ']' Done Line 149: ) Line 150: ) Line 151: self.environment[ Line 152: ovmpcons.ConfigEnv.VMCONSOLE_PROXY_PORT] = port Line 148: ovmpcons.ConfigEnv.VMCONSOLE_PROXY_PORT] Line 149: ) Line 150: ) Line 151: self.environment[ Line 152: ovmpcons.ConfigEnv.VMCONSOLE_PROXY_PORT] = port > here too Done Line 153: break Line 154: except ValueError: Line 155: pass Line 156: Line 299: def exists_user_group(log, user, group): Line 300: try: Line 301: pwd.getpwnam(user) Line 302: except KeyError: Line 303: log.warn(('User {user} does not exist.'.format(user=user))) > Please use '_()' to allow localization Done Line 304: return False Line 305: Line 306: try: Line 307: grp.getgrnam(group) Line 305: Line 306: try: Line 307: grp.getgrnam(group) Line 308: except KeyError: Line 309: log.warn(('Group {group} does not exist.'.format(group=group))) > Here too Done Line 310: return False Line 311: Line 312: return True Line 313: https://gerrit.ovirt.org/#/c/35906/29/packaging/setup/plugins/ovirt-engine-setup/vmconsole_proxy/pki.py File packaging/setup/plugins/ovirt-engine-setup/vmconsole_proxy/pki.py: Line 67: oenginecons.PKIEnv.STORE_PASS > We are already initializing it in packaging/setup/plugins/ovirt-engine-rena removed Line 68: oengcommcons.Defaults.DEFAULT_PKI_STORE_PASS Line 69: ) Line 70: self.environment.setdefault( Line 71: oenginecons.PKIEnv.COUNTRY, Line 72: oengcommcons.Defaults.DEFAULT_PKI_COUNTRY > Here too same Line 73: ) Line 74: self.environment.setdefault( Line 75: oenginecons.PKIEnv.ORG, Line 76: None Line 71: oenginecons.PKIEnv.COUNTRY, Line 72: oengcommcons.Defaults.DEFAULT_PKI_COUNTRY Line 73: ) Line 74: self.environment.setdefault( Line 75: oenginecons.PKIEnv.ORG, > Also here same Line 76: None Line 77: ) Line 78: Line 79: @plugin.event( Line 92: ) and Line 93: self.environment[oenginecons.PKIEnv.ORG] is None Line 94: ), Line 95: ) Line 96: def _customization(self): > I don't think you need to duplicate this code: you already have it if you a Yes, this is a relic of past revisions. Will remove. Line 97: org = 'Test' Line 98: if '.' in self.environment[osetupcons.ConfigEnv.FQDN]: Line 99: org = self.environment[ Line 100: osetupcons.ConfigEnv.FQDN -- To view, visit https://gerrit.ovirt.org/35906 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I034ef8e6d10da5dc93eda61e0c5c518ca13a5a28 Gerrit-PatchSet: 29 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vitor de Lima <vdel...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Vitor de Lima <vdel...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
