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

server.OnDispose from common keys recommendation is missing #207

Closed
binki opened this issue Jun 6, 2018 · 5 comments
Closed

server.OnDispose from common keys recommendation is missing #207

binki opened this issue Jun 6, 2018 · 5 comments

Comments

@binki
Copy link

binki commented Jun 6, 2018

I can’t find in the source code where the server.OnDispose property is supported. I assume that means there is no typed API provided around it.

When I look at http://owin.org/html/CommonKeys.html , it’s there. When I look in this typesafe wrapper API, it appears that the closest thing is

public const string OnAppDisposing = "host.OnAppDisposing";
and
/// <summary>
/// Gets or sets the cancellation token for “host.OnAppDisposing”.
/// </summary>
/// <returns>The cancellation token for “host.OnAppDisposing”.</returns>
public CancellationToken OnAppDisposing
{
get { return Get<CancellationToken>(OwinConstants.CommonKeys.OnAppDisposing); }
set { Set(OwinConstants.CommonKeys.OnAppDisposing, value); }
}
.

I can’t tell from the documentation if the behavior of server.OnDispose and host.OnAppDisposing is supposed to be identical. If it is, it would make sense for a new typed wrapper of server.OnDispose fall back to host.OnAppDisposing to support outdated IIS/SystemWeb. If they have different behaviors, it would be helpful for a new typesafe API for server.OnDispose to be added and the documentation describe the difference in behavior between the two options and maybe to deprecate host.OnAppDisposing.

@Tratcher
Copy link
Member

Tratcher commented Jun 6, 2018

Hmm, I don't know where server.OnDispose came from. It wasn't in the earlier CommonKeys I wrote. Know any servers that implement it?

@Tratcher
Copy link
Member

Tratcher commented Jun 6, 2018

And yes, the described functionality is the same.

@binki
Copy link
Author

binki commented Jun 6, 2018

@Tratcher

Know any servers that implement it?

I don’t. I just stumbled across it at Bobris/Nowin#36 (comment) which states that it is the standard/recommended way to get the token. Maybe that means Nowin implements it, but I haven’t checked it out. I haven’t looked, but I suspect that means it would be good for SystemWeb to be updated to provide it too?

@Tratcher
Copy link
Member

Tratcher commented Jun 6, 2018

Last I checked only SystemWeb provided any such functionality, it'd be more straightforward to update the spec.

@muratg
Copy link

muratg commented Oct 31, 2018

We don't plan to add more features to Katana as it stands now.

@muratg muratg closed this as completed Oct 31, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants