Simone Tiraboschi has posted comments on this change.

Change subject: Avoiding legacy health servlet usage
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.ovirt.org/#/c/26090/2/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/all-in-one/vdsm.py
File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/all-in-one/vdsm.py:

Line 107:                 # to check if the engine is up.
Line 108:                 # Maybe in the future we can just rely on a
Line 109:                 # not authenticated health API URL
Line 110:                 self._ovirtsdk_api.API(
Line 111:                     
url='https://localhost:{port}/ovirt-engine/api'.format(
> you cannot use localhost without insecure parameter... not sure what is the
Done
Line 112:                         port=self.environment[
Line 113:                             osetupcons.ConfigEnv.PUBLIC_HTTPS_PORT
Line 114:                         ],
Line 115:                     ),


Line 120:                     password=self.environment[
Line 121:                         osetupcons.ConfigEnv.ADMIN_PASSWORD
Line 122:                     ],
Line 123:                     ca_file=osetupcons.FileLocations.
Line 124:                         OVIRT_ENGINE_PKI_ENGINE_CA_CERT,
> our convention for this is:
Done
Line 125:                 )
Line 126:                 isUp = True
Line 127:             except:
Line 128:                 time.sleep(self.ENGINE_DELAY)


Line 180:             osetupcons.Stages.APACHE_RESTART,
Line 181:         ),
Line 182:     )
Line 183:     def _closeup(self):
Line 184:         self._waitEngineUp()
> you can return the object from this function and use it in following code..
I absolutely agree with you that is not a good idea to reconstruct it but, in 
my opinion, in the final design we should have the  _waitEngineUp calling a 
fast non authenticated API to wait for the engine to be up, only then we should 
try to construct the SDK.
So I'd prefer to keep this design to limit a future patch to _waitEngineUp only
Line 185:         self.logger.debug('Connecting to the Engine')
Line 186:         engine_api = self._ovirtsdk_api.API(
Line 187:             url='https://localhost:{port}/ovirt-engine/api'.format(
Line 188:                 
port=self.environment[osetupcons.ConfigEnv.PUBLIC_HTTPS_PORT],


Line 183:     def _closeup(self):
Line 184:         self._waitEngineUp()
Line 185:         self.logger.debug('Connecting to the Engine')
Line 186:         engine_api = self._ovirtsdk_api.API(
Line 187:             url='https://localhost:{port}/ovirt-engine/api'.format(
> again... this should have failed or we have a bug in sdk :)
On my machines works without any issue, try! But I agree with you that it's 
better to specify insecure
Line 188:                 
port=self.environment[osetupcons.ConfigEnv.PUBLIC_HTTPS_PORT],
Line 189:             ),
Line 190:             username='{user}@{domain}'.format(
Line 191:                 user=osetupcons.Const.USER_ADMIN,


-- 
To view, visit http://gerrit.ovirt.org/26090
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48225db31b57f70687887f4c06fb923648bfe9f6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: David Caro <dcaro...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@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

Reply via email to