Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check remote host being available. #17835

Merged
merged 2 commits into from
Mar 14, 2017

Conversation

heejaechang
Copy link
Contributor

when host is shutting down it is possible GetRemoteHostClient return null even though host itself support remote hosting.

when host is shutting down it is possible GetRemoteHostClient return null even though host itself support remote hosting.
@heejaechang
Copy link
Contributor Author

@CyrusNajmabadi can you take a look?

@heejaechang
Copy link
Contributor Author

fix #17768

@@ -76,7 +76,11 @@ private class RemoteUpdateEngine : ISymbolSearchUpdateEngine
_sessionDoNotAccessDirectly = null;

_client = await _workspace.GetRemoteHostClientAsync(CancellationToken.None).ConfigureAwait(false);
_client.ConnectionChanged += OnConnectionChanged;
Copy link
Member

Choose a reason for hiding this comment

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

are there other people that might observe that the client returned by GetRemoteHostClientAsync could be null?

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

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

👍
But i'm worried about other people needing to check for this. Can we rename the method to TryGetRemoteHostClient? Making it clear that it may fail and that the caller needs to handle that.

@heejaechang
Copy link
Contributor Author

sure

…it clear that it can return null when remote host is not available.

left GetRemotehostClientAsync there as Obsolete so that this doesnt break other team such as LUT.

will remove it once other team moved to new bits.
@heejaechang
Copy link
Contributor Author

tagging @KevinH-MS I renamed GetRemoteHostClientAsync to TryGetRemoteHostClientAsync

@heejaechang
Copy link
Contributor Author

retest windows_release_vs-integration_prtest please

@heejaechang heejaechang merged commit 15ba5d4 into dotnet:master Mar 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants