Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Remove ListenOptions.Scheme and move IConnectionAdapter to .Internal namespace #1664

Merged
merged 1 commit into from
Apr 13, 2017

Conversation

natemcmaster
Copy link
Contributor

@natemcmaster natemcmaster commented Apr 12, 2017

We know for sure we don't want ListenOptions.Scheme as public API when we release 2.0, so let's get rid of it now.

This fixes #1296.

Resolves #1495

This is obviously a temporary fix as the design of connection adapters and configuration will churn before release. I will open a follow-up issue to create a better design.

@natemcmaster
Copy link
Contributor Author

image

@natemcmaster natemcmaster changed the title Remove ListenOptions.Scheme Remove ListenOptions.Scheme and move IConnectionAdapter to .Internal namespace Apr 12, 2017
@natemcmaster
Copy link
Contributor Author

🆙 📅 - moved IConnectionAdapter to .Internal and add bool IsHttps

@@ -145,6 +145,10 @@ public void Start<TContext>(IHttpApplication<TContext> application)
{
throw new InvalidOperationException($"HTTPS endpoints can only be configured using {nameof(KestrelServerOptions)}.{nameof(KestrelServerOptions.Listen)}().");
}
else if (!parsedAddress.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase))
{
throw new InvalidOperationException($"Unrecognized scheme in server address '{address}'. Only 'http://' is supported.");
Copy link
Member

Choose a reason for hiding this comment

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

Is there a test for this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is now :)

Copy link
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

LGTM

@natemcmaster natemcmaster force-pushed the namc/server-addresses branch from 4628162 to 1cf6858 Compare April 13, 2017 16:42
…amespace

Add an internal API to ListenOptions to determine if an endpoint is configured to use HTTPS. This is a temporary as the design of connection adapters and configuration will churn before release.
@natemcmaster natemcmaster force-pushed the namc/server-addresses branch from 1cf6858 to ed4a27a Compare April 13, 2017 16:46
@natemcmaster natemcmaster merged commit ed4a27a into dev Apr 13, 2017
@natemcmaster natemcmaster deleted the namc/server-addresses branch April 13, 2017 17:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants