Remote console integration #113
Conversation
| @@ -203,7 +203,8 @@ contextBridge.exposeInMainWorld( | |||
| 'get-wrc-home-directory', | |||
| 'get-wrc-app-image', | |||
| 'wrc-get-home-default-value', | |||
| 'wrc-set-home-and-start' | |||
| 'wrc-set-home-and-start', | |||
| 'get-wrc-port' | |||
There was a problem hiding this comment.
As we discussed a few weeks back, I don't understand the purpose of this IPC call. The electron side is getting the port number and pushing it to the webui side once it is available. Making the webui side pull the port number doesn't eliminate the asynchronous nature of this and your code still has to be prepared to get undefined back since your code may call this method before the backend has started and the port is known.
My sense is that you think you are eliminating the asynchrony because you haven't seen the race condition but rest assured that you are not eliminating the race condition by doing this.
There was a problem hiding this comment.
I was not thinking that the get-wrc-port IPC eliminated the race condition. In fact, lines 51-53 in the model-design-view.js model look for whether the port number sent to windowStateUtils.js was undefined, or not.
The get-wrc-port IPC is there because there scenarios involving the WRC, which could leave the WKT-UI/WRC piece in an unstable state. For instance, if the spawned WRC app dies (or literally gets killed by the end user) and the user is interacting with the "Design View" tab, then the get-wrc-port IPC is the way to gracefully recover. To see this, click on the "Design View" tab, kill the process associated with the started WRC app, then change a field in the WRC form or click on the navtree. The get-wrc-port IPC is the reason you end up back on the "WRC not installed" version of the "Design View" tab, and not dangling in a state where the embedded WRC is still trying to make REST calls to the CBE. You have it so the electron side prints the exit code of the killed WRC, but that doesn't 1) help the embedded WRC know that the backend is now invalid, and 2) set things up so that the "WRC not installed" version of the "Design View" tab gets displayed.
There was a problem hiding this comment.
Here's the stdout showing log entries from both the electron and webui side, in the scenario described above:
info: [renderer:2] showWdtModelDesigner: backendPort=52301
info: WebLogic Remote Console backend process exited with code 0
info: [renderer:2] showWdtModelDesigner: backendPort=undefined
info: [renderer:2] showWdtModelDesigner: backendPort=52340
The get-wrc-port IPC is what's allowing the model-design-view.js to figure out what's going on, and communicate the valid backendPort to the wdt-model-designer JET composite, so it can in turn set the valid backendURL to be used when doing CBE REST calls.
There was a problem hiding this comment.
So rather than this elaborate scheme that requires a new IPC every time the user switches to the Design View tab, why wouldn’t we just make the call to push the new value of the port when the process exits?
| define(['accUtils', 'utils/i18n', 'knockout', 'models/wkt-project', 'utils/url-catalog', 'utils/wkt-logger', 'wrc-frontend/core/parsers/yaml', 'wrc-frontend/integration/viewModels/utils', | ||
| 'wdt-model-designer/loader', 'ojs/ojinputtext', 'ojs/ojlabel', 'ojs/ojbutton', 'ojs/ojformlayout'], | ||
| function(accUtils, i18n, ko, project, urlCatalog, wktLogger, YamlParser, ViewModelUtils) { | ||
| function ModelDesignViewModel() { |
There was a problem hiding this comment.
hmm... We already are pulling in js-yaml and using it everywhere, is there something fundamentally different about wrc-frontend/core/parsers/yaml? I am concerned because using 2 different parsers may result in subtle bugs caused by parser differences. Seems like we should use the same parser...
There was a problem hiding this comment.
We used the yaml parser in wrc-jet-pack, because we didn't know which WKT-UI module you guys use to parse yaml. Switching to use whatever the WKT-UI is using should the relatively easy to do.
| // Get WRC backend port, asynchronously | ||
| window.api.ipc.invoke('get-wrc-port') | ||
| .then(backendPort => { | ||
| if (backendPort !== project.wdtModel.internal.wlRemoteConsolePort()) { |
There was a problem hiding this comment.
This is where the race condition I was speaking about earlier exists and you are not handling it (i.e., what if backendPort comes back as undefined)?
Frankly, I would just delete all of this code after line 25 to the end of the function.
There was a problem hiding this comment.
See my previous comment on the scenario the get-wrc-port IPC is there to handle.
|
Superseded by PR #114 |
No description provided.