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

Fix localTrainingService stream #885

Merged
merged 67 commits into from
Mar 26, 2019
Merged

Conversation

SparkSnail
Copy link
Contributor

@SparkSnail SparkSnail commented Mar 19, 2019

fix issue #867

@QuanluZhang QuanluZhang requested a review from goooxu March 21, 2019 09:09
throw new Error(`Could not find stream in trial ${trialJob.id}`);
}
stream.destroy();
this.jobStreamMap.delete(trialJob.id);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a good place to destroy job stream , it is better to put it into onTrialJobStatusChanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to onTrialJobStatusChanged.

throw new Error(`Could not find stream in trial ${trialJob.id}`);
}
stream.destroy();
this.jobStreamMap.delete(trialJob.id);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is not specific for GPU, looks like should move to parent class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to parent class

@SparkSnail SparkSnail merged commit bd34681 into microsoft:master Mar 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants