Skip to content

Change services to local and remote exclusive only#3710

Merged
sbanni merged 1 commit intodevfrom
dev-scoban-liveshare
Oct 9, 2020
Merged

Change services to local and remote exclusive only#3710
sbanni merged 1 commit intodevfrom
dev-scoban-liveshare

Conversation

@sbanni
Copy link
Contributor

@sbanni sbanni commented Oct 8, 2020

Bug

Fixes: NuGet/Home#10117

Fix

Details: Remove LiveShare support for our brokered services

Testing/Validation

Tests Added: No
Validation: Installed locally and tested local (same process) and local remote, same machine two instances of devenv, one server one client. Verified Tools -> Options worked and PM UI scenarios including installing/updating/uninstalling packages.

@sbanni sbanni requested review from dtivel and rrelyea October 8, 2020 21:05
@sbanni sbanni requested a review from a team as a code owner October 8, 2020 21:05
@sbanni sbanni self-assigned this Oct 8, 2020
@rrelyea
Copy link
Contributor

rrelyea commented Oct 8, 2020

@sbanni - i looked for docs on that enum...but doesn't do much explaining...
https://docs.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.shell.servicebroker.serviceaudience?view=visualstudiosdk-2019

can you point towards some descriptions of those different choices...and why you chose what you did.

@sbanni
Copy link
Contributor Author

sbanni commented Oct 8, 2020

@sbanni - i looked for docs on that enum...but doesn't do much explaining...
https://docs.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.shell.servicebroker.serviceaudience?view=visualstudiosdk-2019

can you point towards some descriptions of those different choices...and why you chose what you did.

@rrelyea
The previous entry is defined as this:
AllClientsIncludingGuests = Local | RemoteExclusiveClient | LiveShareGuest,

Best place to find the descriptions for these values is in the code:

    /// <summary>
    /// Services are available for clients running in the same process (or <see cref="AppDomain" /> on the .NET Framework).
    /// They will not be available from other processes (e.g. ServiceHub services).
    /// A brokered service that includes this flag may still be *indirectly* exposed to <see cref="LiveShareGuest"/>
    /// by way of another brokered service that is exposed to <see cref="LiveShareGuest"/> that is proffered from this process.
    /// </summary>
    Process = 0x1,
    
    /// <summary>
    /// The service is available for clients that support this process (e.g. ServiceHub services). These always run on the same machine and user account.
    /// It does *not* include processes connected over Live Share or a Visual Studio Online Environment connection, even if these processes are running on the same machine.
    /// A brokered service that includes this flag may still be *indirectly* exposed to <see cref="LiveShareGuest"/>
    /// by way of another brokered service that is exposed to <see cref="LiveShareGuest"/> that is proffered from this machine.
    /// </summary>
    Local = Process | 0x2,

    /// <summary>
    /// When the service is running on an Visual Studio Online environment it is available to the client.
    /// </summary>
    /// <remarks>
    /// Host services are available for the *one* client running on any machine that is connected remotely using the exclusive
    /// owner connection (not the traditional Live Share sharing session).
    /// Such a connection is *always* owned by the same owner as the server and thus is considered trusted.
    /// </remarks>
    RemoteExclusiveClient = 0x100,

@rrelyea
Copy link
Contributor

rrelyea commented Oct 9, 2020

RemoteExclusiveClient sounds like we should disable it as well. Perhaps double check with Andrew? (doc comments still have room for improvement...to make clearer)

@rrelyea
Copy link
Contributor

rrelyea commented Oct 9, 2020

from scott:
first thing I noticed is that in LiveShare session right now, "Manage NuGet packages" context menu option is available, clicking on it does nothing
With the change, it still does nothing
So self-liveshare does not enable the PM UI
so will review now.

[ProvideBrokeredService(BrokeredServicesUtilities.SolutionManagerServiceName, BrokeredServicesUtilities.SolutionManagerServiceVersion, Audience = ServiceAudience.AllClientsIncludingGuests)]
[ProvideBrokeredService(BrokeredServicesUtilities.ProjectManagerServiceName, BrokeredServicesUtilities.ProjectManagerServiceVersion, Audience = ServiceAudience.AllClientsIncludingGuests)]
[ProvideBrokeredService(BrokeredServicesUtilities.ProjectUpgraderServiceName, BrokeredServicesUtilities.ProjectUpgraderServiceVersion, Audience = ServiceAudience.AllClientsIncludingGuests)]
[ProvideBrokeredService(BrokeredServicesUtilities.SourceProviderServiceName, BrokeredServicesUtilities.SourceProviderServiceVersion, Audience = ServiceAudience.Local | ServiceAudience.RemoteExclusiveClient)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should Local be included here?

Copy link
Contributor Author

@sbanni sbanni Oct 9, 2020

Choose a reason for hiding this comment

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

No, those services are only used in the VS.Oe scenario.

@sbanni sbanni merged commit b828348 into dev Oct 9, 2020
@sbanni sbanni deleted the dev-scoban-liveshare branch October 9, 2020 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove liveshare flag from service attributes

3 participants