Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Delete redundant code #3526

Merged
merged 4 commits into from
Apr 30, 2021
Merged

Delete redundant code #3526

merged 4 commits into from
Apr 30, 2021

Conversation

maxsuren
Copy link
Contributor

Delete redundant code in:
ts/nni_manager/training_service/kubernetes/frameworkcontroller/frameworkcontrollerTrainingService.ts
ts/nni_manager/training_service/kubernetes/kubeflow/kubeflowTrainingService.ts

These lines will never be reached

@ghost
Copy link

ghost commented Apr 13, 2021

CLA assistant check
All CLA requirements met.

@@ -65,7 +65,6 @@ class FrameworkControllerTrainingService extends KubernetesTrainingService imple
await this.fcJobInfoCollector.retrieveTrialStatus(this.kubernetesCRDClient);
if (this.kubernetesJobRestServer.getErrorMessage !== undefined) {
throw new Error(this.kubernetesJobRestServer.getErrorMessage);
this.stopping = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems better to move this in front of the throw new Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello J-shang, I thought about this at the beginning, but than I realized this.stopping never been assigned to true in the old logic, so if I move this.stopping=true to the front of throw new Error, this.stopping will be true when throwing Error and the old logic will be broken.

@ultmaster ultmaster merged commit a2146d5 into microsoft:master Apr 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants