Alon Bar-Lev has posted comments on this change.

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


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/26090/1/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 100:         return isUp
Line 101: 
Line 102:     def _waitEngineUp(self):
Line 103:         self.logger.debug('Waiting for API to check Engine health 
status')
Line 104:         api_url = 'http://{fqdn}:{port}/ovirt-engine/api'.format(
> Is it always safe tu use localhost instead of {fqdn} in this phase?
yes, you avoid to use dns and such, but it should synced with other uses of the 
sdk, which can all be modified to use localhost :)
Line 105:             fqdn=self.environment[osetupcons.ConfigEnv.FQDN],
Line 106:             
port=self.environment[osetupcons.ConfigEnv.PUBLIC_HTTP_PORT],
Line 107:         )
Line 108:         tries = self.ENGINE_RETRIES


Line 109:         isUp = False
Line 110:         while not isUp and tries > 0:
Line 111:             tries -= 1
Line 112:             try:
Line 113:                 with contextlib.closing(urllib2.urlopen(api_url)) as 
urlObj:
> As far as I understand, the whole _waitEngineUp() is used to wait for the e
SDK and API are two separate terms/components.

we are waiting for engine to be up.

we query the api.

we use sdk to interact with api.

urllib is bad as it does not support redirect and may have other issues, like 
what you require now to use ssl and ignore certificates.

the sdk also will raise correct errors if something goes wrong.

please do not re-implement what already implemented.
Line 114:                     content = urlObj.read()
Line 115:                     if content:
Line 116:                         if self.PI_RE.match(content) is None:
Line 117:                             raise RuntimeError(


-- 
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: David Caro <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Simone Tiraboschi <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to