microsoft/2LCS

RDP instant mode doesn't work

DjLysy opened this issue · 5 comments

DjLysy commented

Since making the refresh environments methods working in async mode, RDP instant mode when launching the 2LCS with parameters doesn't work anymore

Example: execute the following path:
.\2LCS.exe ms-2lcs://connectRDP/?projectId=(projectId)&environment=(envName)&user=Admin

It will fail on calling HttpClientHelper.GetCheInstancesAsync()

Thanks for reporting the issue and providing the fix in pr #94 as well, much appreciated! I merged it and released 0.49.0.

I was not even aware that 2LCS could be used like that. Seems something that we should add to the documentation.

Just to provide context for the future, this functionality was added in #66, while the async stuff was added in #75

@fraga Let us know if you see any issues in undoing async for RDPConnect.

fraga commented

Hey this is great!

What if we add the sync version as it was and just wrap that in async instead of just removing the async? Let me know your thoughts.

Could you explain a bit more on how this could be done? I'm a bit out of my depth with C# and async.

From what I can tell, since BackgroundWorker is used to call OnWorkerDoWork, it already runs asynchronously?

worker = new BackgroundWorker
{
WorkerReportsProgress = true,
WorkerSupportsCancellation = true
};
worker.DoWork += new DoWorkEventHandler(OnWorkerDoWork);
worker.ProgressChanged += new ProgressChangedEventHandler(OnWorkerProgressChanged);
worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(OnWorkerRunWorkerCompleted);
this.SetProgress(10, "Authenticating to LCS");
using (var form = new Login())
{
form.ShowDialog();
}
worker.RunWorkerAsync();

fraga commented

Never mind I overlooked the implementation and when I did the async piece that should've left as sync and we would still use the Result as @DjLysy implemented is correct.

Looking forward, I think we need to refactor the backgroundworker and replace it with Task. Since this is all probably going to go away with One experience (one admin) I would leave it for now!

Thanks @DjLysy for the contribution!

PS: I'm always out of my depth with C# too, always learning!

@fraga Thanks for looking into it. I agree, the LCS days are numbered and so is 2LCS. But while we still have it, it is work keeping it alive. Will keep the Task refactor in mind in case we revisit that part again.