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

Added support for unix domain sockets to the sockets transport #10560

Merged
merged 1 commit into from May 31, 2019

Conversation

@davidfowl
Copy link
Member

commented May 27, 2019

  • Added a test for UnixDomainSockets on both transports

Fixes #4768
Fixes #4764

PS: This is on top of #10321. This will be rebased once that's in.

@davidfowl davidfowl requested a review from halter73 May 27, 2019

@davidfowl davidfowl requested review from anurse, jkotalik and Tratcher as code owners May 27, 2019

@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 311a23a to 803cb93 May 28, 2019

@Eilon Eilon added the area-servers label May 28, 2019

@anurse anurse added this to the 3.0.0-preview7 milestone May 28, 2019

@anurse

This comment has been minimized.

Copy link
Member

commented May 28, 2019

@davidfowl Do you expect to be able to get this in and verified e2e by Friday? Assuming not, we should wait until preview 7 (opens next week) to merge.

@davidfowl

This comment has been minimized.

Copy link
Member Author

commented May 28, 2019

@davidfowl Do you expect to be able to get this in and verified e2e by Friday? Assuming not, we should wait until preview 7 (opens next week) to merge.

Yes, I expect it to be in today or tomorrow.

@anurse anurse modified the milestones: 3.0.0-preview7, 3.0.0-preview6 May 28, 2019

@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 803cb93 to 90ca693 May 29, 2019

});
})
.ConfigureServices(AddTestLogging)
.Configure(c => { });

This comment has been minimized.

Copy link
@halter73

halter73 May 29, 2019

Member

Out of curiosity, is this line necessary?

This comment has been minimized.

Copy link
@davidfowl

davidfowl May 29, 2019

Author Member

Yea, the webhost throws without it.

@@ -31,10 +31,6 @@ internal sealed class SocketConnectionListener : IConnectionListener
SocketTransportOptions options,
ISocketsTrace trace)
{
Debug.Assert(endpoint != null);

This comment has been minimized.

Copy link
@jkotalik

jkotalik May 29, 2019

Member

endpoint != null is still a valid assert I believe.

[ConditionalFact]
public async Task TestUnixDomainSocket()
{
var path = Path.GetTempFileName();

This comment has been minimized.

Copy link
@halter73

halter73 May 29, 2019

Member

This seems to be creating the file.

This comment has been minimized.

Copy link
@jkotalik

jkotalik May 31, 2019

Member

Yeah this is a bit jank.

This comment has been minimized.

Copy link
@anurse

anurse May 31, 2019

Member

Especially when you're using it just to get a name and then immediately deleting it. This is a bit of a flake-farm waiting to happen... why not just use Path.Combine(Path.GetTempPath(), $"Kestrel_TestUnixDomainSocket_{Guid.NewGuid():N}.socket") or something?

@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 90ca693 to 47917d6 May 30, 2019

Added support for unix domain sockets to the sockets transport
- Added a test for UnixDomainSockets on both transports

@davidfowl davidfowl changed the base branch from davidfowl/transport-refactor to master May 31, 2019

@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 47917d6 to b3f638c May 31, 2019

@davidfowl

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

@halter73 @jkotalik is this good to go?

@davidfowl davidfowl merged commit ecacf90 into master May 31, 2019

15 of 16 checks passed

AspNetCore-helix-test Build #206656 failed
Details
AspNetCore-ci Build #20190530.58 succeeded
Details
AspNetCore-ci (Build: Linux ARM) Build: Linux ARM succeeded
Details
AspNetCore-ci (Build: Linux ARM64) Build: Linux ARM64 succeeded
Details
AspNetCore-ci (Build: Linux Musl ARM64) Build: Linux Musl ARM64 succeeded
Details
AspNetCore-ci (Build: Linux Musl x64) Build: Linux Musl x64 succeeded
Details
AspNetCore-ci (Build: Linux x64) Build: Linux x64 succeeded
Details
AspNetCore-ci (Build: Windows ARM) Build: Windows ARM succeeded
Details
AspNetCore-ci (Build: Windows x64/x86) Build: Windows x64/x86 succeeded
Details
AspNetCore-ci (Build: macOS) Build: macOS succeeded
Details
AspNetCore-ci (Code check) Code check succeeded
Details
AspNetCore-ci (Test: Templates - Windows Server 2016 x64) Test: Templates - Windows Server 2016 x64 succeeded
Details
AspNetCore-ci (Test: Ubuntu 16.04 x64) Test: Ubuntu 16.04 x64 succeeded
Details
AspNetCore-ci (Test: Windows Server 2016 x64) Test: Windows Server 2016 x64 succeeded
Details
AspNetCore-ci (Test: macOS 10.13) Test: macOS 10.13 succeeded
Details
license/cla All CLA requirements met.
Details

@davidfowl davidfowl deleted the davidfowl/unix-domain-sockets branch Jul 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.