Alon Bar-Lev has posted comments on this change.
Change subject: engine: [WIP] Integrate noVNC support
......................................................................
Patch Set 4: (5 inline comments)
....................................................
File packaging/conf/ovirt-websocket-proxy.conf.defaults.in
Line 12: FORCE_DATA_VERIFICATION=False
Line 13: CERT_FOR_DATA_VERIFICATION=
Line 14: SSL_ONLY=False
Line 15: DAEMON=False
Line 16: RECORD=False
:) so LOG_FILE? and drop ENGINE_LOG...
Line 17: VERBOSE=False
Line 18: ENGINE_USR="@ENGINE_USR@"
Line 19: ENGINE_LOG="@ENGINE_LOG@"
Line 20: WSPROXY_STOP_TIME=30
....................................................
File packaging/services/ovirt-websocket-proxy.py
Line 77: self._engineEnv = os.environ.copy()
Line 78: self._engineEnv.update({
Line 79: 'PATH':
'/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin',
Line 80: 'ENGINE_WSPROXY_DEFAULT_FILE':
config.ENGINE_WSPROXY_DEFAULT_FILE,
Line 81: 'ENGINE_WSPROXY_VARS': config.ENGINE_WSPROXY_VARS,
This depends on the implementation...
In order to read configuration file you need to use the self._config, which is
service.ConfigFile. This reads the files (default, config, config.d/*), merge
them into configuration.
Then you can query configuration just like you done bellow.
Passing the name of the configuration file to other process without using
ConfigFile to load it does not help you... as you cannot read the vars properly.
If you move the entire code here, you can simply use the self._config... which
was what I expected... but if you don't you should read the variables, place
the KEY=VALUES in environment and execute the other process.
Line 82: })
Line 83:
Line 84: def daemonStdHandles(self): # TODO - do i need this?
Line 85: consoleLog = open(
Line 89: 'websocket-proxy.log'
Line 90: ),
Line 91: 'w+',
Line 92: )
Line 93: return (consoleLog, consoleLog)
There should be one log... one process. The problem is mostly because of the
split...
Line 94:
Line 95: def daemonContext(self):
Line 96: self.daemonAsExternalProcess(
Line 97: executable=self._executable,
Line 92: )
Line 93: return (consoleLog, consoleLog)
Line 94:
Line 95: def daemonContext(self):
Line 96: self.daemonAsExternalProcess(
It is already split into the Service class.. whatever in this file is websocket
proxy specific...
I do not see any reason to have wrapper of python to python...
A wrapper of java to python was actually enforced...
Line 97: executable=self._executable,
Line 98: args=self._args,
Line 99: env=self._engineEnv,
Line 100: stopTime=self._config.getInteger(
....................................................
File packaging/services/ovirt-websocket-proxy.sysv.in
Line 33: echo $"Insufficient privilege" 1>&2
Line 34: exit 4
Line 35: fi
Line 36: echo -n $"Starting $PROG: "
Line 37: ulimit -n ${FILENO:-65535}
when you decide about the limit this should be modified as well.
Line 38: touch "${PIDFILE}"
Line 39: chown "${USER}" "${PIDFILE}"
Line 40: daemon --user "${USER}" --pidfile="${PIDFILE}" \
Line 41:
"${ENGINE_USR}/services/ovirt-websocket-proxy.py" \
--
To view, visit http://gerrit.ovirt.org/13931
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I44e9870b88537360a1886e89c08f18865eae2ef0
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Martin Beták <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
