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

@davidfowl davidfowl 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 force-pushed the davidfowl/unix-domain-sockets branch from 311a23a to 803cb93 May 28, 2019
@anurse anurse added this to the 3.0.0-preview7 milestone May 28, 2019
@anurse
Copy link
Contributor

@anurse anurse 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.

Loading

@davidfowl
Copy link
Member Author

@davidfowl davidfowl 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.

Loading

@anurse anurse removed this from the 3.0.0-preview7 milestone May 28, 2019
@anurse anurse added this to the 3.0.0-preview6 milestone May 28, 2019
@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 803cb93 to 90ca693 May 29, 2019
});
})
.ConfigureServices(AddTestLogging)
.Configure(c => { });
Copy link
Member

@halter73 halter73 May 29, 2019

Choose a reason for hiding this comment

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

Out of curiosity, is this line necessary?

Loading

Copy link
Member Author

@davidfowl davidfowl May 29, 2019

Choose a reason for hiding this comment

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

Yea, the webhost throws without it.

Loading

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

@jkotalik jkotalik May 29, 2019

Choose a reason for hiding this comment

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

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

Loading

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

@halter73 halter73 May 29, 2019

Choose a reason for hiding this comment

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

This seems to be creating the file.

Loading

Copy link
Member

@jkotalik jkotalik May 31, 2019

Choose a reason for hiding this comment

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

Yeah this is a bit jank.

Loading

Copy link
Contributor

@anurse anurse May 31, 2019

Choose a reason for hiding this comment

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

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?

Loading

@davidfowl davidfowl force-pushed the davidfowl/unix-domain-sockets branch from 90ca693 to 47917d6 May 30, 2019
- 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
Copy link
Member Author

@davidfowl davidfowl commented May 31, 2019

@halter73 @jkotalik is this good to go?

Loading

@davidfowl davidfowl merged commit ecacf90 into master May 31, 2019
15 of 16 checks passed
Loading
@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
Linked issues

Successfully merging this pull request may close these issues.

6 participants