kunwp1 commented on code in PR #3773:
URL: https://github.com/apache/texera/pull/3773#discussion_r2384272778
##########
core/gui/src/app/workspace/service/computing-unit-status/computing-unit-status.service.ts:
##########
@@ -248,9 +266,13 @@ export class ComputingUnitStatusService implements
OnDestroy {
public terminateComputingUnit(cuid: number): Observable<boolean> {
const isSelected = this.selectedUnitSubject.value?.computingUnit.cuid ===
cuid;
- if (isSelected && this.workflowWebsocketService.isConnected) {
- this.workflowWebsocketService.closeWebsocket();
- this.workflowStatusService.clearStatus();
+ if (isSelected) {
Review Comment:
Are these changes necessary? Based on your description of the PR, I don't
see the reason why you are changing the `terminateComputingUnit`.
##########
core/gui/src/app/workspace/service/computing-unit-status/computing-unit-status.service.ts:
##########
@@ -116,6 +117,8 @@ export class ComputingUnitStatusService implements
OnDestroy {
// The selected unit is no longer in the list
this.selectedUnitSubject.next(null);
this.stopPollingSelectedUnit();
+ this.currentConnectedCuid = undefined;
Review Comment:
Are these changes necessary? Based on your description of the PR, I’m not
sure they are.
##########
core/gui/src/app/workspace/service/computing-unit-status/computing-unit-status.service.ts:
##########
@@ -153,17 +156,32 @@ export class ComputingUnitStatusService implements
OnDestroy {
*/
public selectComputingUnit(wid: number | undefined, cuid: number): void {
const trySelect = (unit: DashboardWorkflowComputingUnit) => {
- // open websocket if needed
- if (isDefined(wid) && this.currentConnectedCuid !== cuid) {
+ if (!isDefined(wid)) {
if (this.workflowWebsocketService.isConnected) {
this.workflowWebsocketService.closeWebsocket();
this.workflowStatusService.clearStatus();
}
+ this.currentConnectedCuid = undefined;
+ this.currentConnectedWid = undefined;
+ this.selectedUnitSubject.next(unit);
Review Comment:
Are these changes necessary? I don't understand why we need to worry about
closing the websocket connection when `wid` is undefined. Can you explain the
scenario where `wid` becomes undefined in `selectComputingUnit`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
