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

Move IAppBuilder to Microsoft.Owin #19

Closed
panesofglass opened this issue Dec 2, 2013 · 175 comments
Closed

Move IAppBuilder to Microsoft.Owin #19

panesofglass opened this issue Dec 2, 2013 · 175 comments

Comments

@panesofglass
Copy link
Member

IAppBuilder is not part of the OWIN spec and should not be included in an owin.dll. This just confuses people new to OWIN.

@tugberkugurlu
Copy link

-1. If it's going to be moved, Project Katana is not the place for that.

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

I agree it causes some confusion, but IAppBuilder can't be moved without resetting the ecosystem. I also wouldn't want to move it until there was an official replacement, so we could avoid breaking the world twice.

I also agree with Tugber, Microsoft.Owin is not the pace to put it. Owin Contrib, in something named Owin.AppBuilder might work.

@panesofglass
Copy link
Member Author

What uses this outside Katana? It is, as far as I know, Katana specific.

@panesofglass
Copy link
Member Author

Also, there can be no replacement for an undefined spec. That this assembly exists is an historical accident/mistake.

@half-ogre
Copy link

I see two problems here:

  1. IAppBuilder has become the default "spec" for an OWIN builder. We really need a proper spec for a builder that is as flexible as OWIN itself (e.g., doesn't favor class construction as a means for context).
  2. That builder interface is being shipped in a DLL and NuGet package named OWIN, when it's not OWIN but a builder interface.

I'd like to see a proper builder spec written so we have a consistent way to use middleware with hosts. And, like OWIN, it shouldn't be dependent on type creation (but neither should prevent it). Renaming the DLL and the NuGet package would be good, but it doesn't solve the actual problem of IAppBuilder being a poor "spec". It should really be removed completely once a proper builder spec is produced. And if we want a rich host ecosystem, we need a proper builder spec so we're all consistently building our middlewares.

@panesofglass
Copy link
Member Author

This is a simple matter of versioning. Owin.dll 2.0 = empty assembly :)

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

How about taking this to the owin discussion list instead?

@panesofglass
Copy link
Member Author

I disagree that we need a builder spec. I agree we should finally define a standard for chaining these things, since that seems to be a pain point for some.

Mark's delegate works, as does

Func<AppFunc, AppFunc>

@tugberkugurlu
Copy link

@panesofglass
forgive my ignorance but what problem do we solve if we don't have a builder spec?

+1 to @half-ogre IMO. Not having a really concrete spec for builder is what is the cause of the problem. Even making the IAppBuilder a spec would solve 90% of the problems (I agree that it's not ideal, though).

@half-ogre
Copy link

I agree we should finally define a standard for chaining these things, since that seems to be a pain point for some.

That's a builder spec.

@panesofglass
Copy link
Member Author

@tugberkugurlu why do we need a builder? That is an implementation detail, not part of OWIN.

@half-ogre
Copy link

How about taking this to the owin discussion list instead?

I'm obviously biased given where I work, but I much prefer to keep things on a repo where the context lives.

@panesofglass
Copy link
Member Author

I agree with @half-ogre

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

How much of the community is represented here? This affects everybody.

@panesofglass
Copy link
Member Author

@Tratcher Most people are using Microsoft.Owin. I don't understand how moving this interface to Microsoft.Owin breaks people. Everything in Owin.Extensions already moved. How is this different?

Also, please note that we (inclusive of those on the Twitter discussion) are not saying IAppBuilder is evil, bad, etc. It's a Microsoft-specific solution that works just fine within the Katana ecosystem. It's just not a standard for OWIN and shouldn't be owned by nor packaged by OWIN proper. It doesn't work for those of us building alternate implementations and creates a lot of confusion.

If this can't get resolved, it is my opinion that the "Open" in OWIN no longer applies, and we (the community) will have to start working on the next OWIN since this effort appears to have been hijacked.

@prabirshrestha
Copy link

+1 for Func<AppFunc, AppFunc> or any other solution that does not include IAppBuilder.
+1 for discussing it here. Might be someone should open post a new message in google groups and notify others to watch this repo.

@tugberkugurlu
Copy link

@panesofglass MSFT has been putting a lot effort on OWIN based software and building great solutions. Honestly, I don't think I would care that much about the-next-OWIN that "the community" (which I'm not part of apperently :)) will work on.

@panesofglass
Copy link
Member Author

As for who is involved, the Twitter discussion included @skoon, @grumpydev, @serialseb, @darrelmiller, @jeremydmiller, @markrendle, and others, covering NancyFx, OpenRasta/OpenHttp, Fubu, Fix/Simple.Web, and Dyfrig/Frank (F#). I'd say that's covering most of the framework community. @markrendle and I also have server libraries that are trying to support OWIN.

@tugberkugurlu
Copy link

@panesofglass I need a builder spec so that the host that I'm crafting knows how to wire up the stuff.

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

IAppBuilder was one of the early community proposals, it's not MS specific, it's just the one MS adopted. There are already MS independent implementations out there. It did unfortunately end up in Owin.dll without first being officially approved by the community.

IAppBuilder can be moved to help clear up the confusion, but doing so is going to be a big break because .NET ties type names to assemblies. If we're going to make that break, I'd prefer to have a community replacement ready to move to so we only have to break the world once.

@panesofglass
Copy link
Member Author

@tugberkugurlu Great! Why can't that exist in Katana? It's a great implementation. What's requiring you have that in an Owin.dll?

@panesofglass
Copy link
Member Author

@Tratcher That was @loudej's recommendation. That was never accepted by the community at large. I recall the phone discussions acknowledging that was fine so long as it was not spec. Much of Gate moved to Katana. @bvanderveen helped, of course, but that was all still in the wild west days of the "Delegate of Doom" ™️

I understand now the impact. Thanks. I'm happy to make the move once we settle on a common replacement.

@tugberkugurlu
Copy link

@panesofglass Having a stable software is one thing that I'm all for. If the community you are talking about releases a software which doesn't step outside of the v0.* for years, I'll stick with the MSFT solutions which seem to be pretty robust and have stable release cycles.

I don't think spiting the commutiy into two will solve any of these problems and I don't also think most of the people will care about only community driven the-next-OWIN.

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

Assuming the community can finally standardize the startup sequence, there's no reason Katana would need to keep Owin.dll / IAppBuilder either. We use it for lack of a standard alternative. We don't want to fork the community either.

@tugberkugurlu
Copy link

@panesofglass as @half-ogre said:

I agree we should finally define a standard for chaining these things, since that seems to be a pain point for some.

that's the builder spec.

@half-ogre
Copy link

@panesofglass as @half-ogre said:

I agree we should finally define a standard for chaining these things, since that seems to be a pain point for some.
that's the builder spec.

To be clear, I'm not saying we should specify how a builder is implemented. I'm saying we should specify how a middleware is used in a builder, in the same way OWIN specifies how an app is used (the app delegate).

@panesofglass
Copy link
Member Author

@Tratcher We are in agreement there. I think specifying the startup sequence would be very useful. However, it still has to be fairly generic.

@tugberkugurlu I think it's terrific to use the Microsoft sponsored bits. I've already used Katana on several projects at work, and I'm about to do so again. This is hitting up against another issue, which is interop among OWIN implementations. Most fell behind b/c the v1.0 spec launched forward, and Katana was the first to get there. The rest of us have full-time jobs other than building frameworks. That doesn't mean we should have to fall in line with whatever the first mover decides.

OWIN started as a community-driven solution to a common problem for framework and server devs working on side-projects part-time. Microsoft's involvement is highly prized and appreciated. However, that doesn't mean that they now own OWIN.

We, the community, were willing to break compat with ASP.NET and IIS. I think we would be willing to do so again. I don't think any of us want to do that, but it's not really an issue for us since most of us have far smaller install bases.

Honestly, I am not sure what you are arguing for. You seem to like Katana, and that's great. If Katana satisfies, then what are you proposing? Are you trying to build your own implementation? Do you want to use other implementations? I'm quite confused.

@Tratcher
Copy link
Contributor

Tratcher commented Dec 2, 2013

So, are we official re-convening the OWIN community to address startup? Finally?

If so, I might also suggest reviving the conference calls. They were very useful the first time around for getting everybody on the same page.

@prabirshrestha
Copy link

For reference: Here is a discussion from last year about the IAppBuilder - IAppBuilder in the OWIN spec https://groups.google.com/forum/#!searchin/net-http-abstractions/IAppBuilder/net-http-abstractions/Xy2l71SmYsQ/CUA6VJxDil4J

@skoon
Copy link
Contributor

skoon commented May 12, 2014

As of today, the proposal to sunset the OWIN.dll containing IAppBuilder is passing 13-0
(+1: 9, 0: 4, -1:0)

I think we can close the voting on this and discuss revamping the documents to provide a way for users of IAppBuilder to migrate to the current spec?

@markrendle
Copy link

Good stuff.

Some time this week I'm going to find a minute to submit a pull request to
Raygun.Owin removing the Owin.dll dependency and write a blog post about
it. Possibly some of the text from that could be used in the docs?

M

Mark Rendle
Founder, Zudio https://zud.io/

On Mon, May 12, 2014 at 5:07 PM, Scott Koon notifications@github.comwrote:

As of today, the proposal to sunset the OWIN.dll containing IAppBuilder is
passing 13-0
(+1: 9, 0: 4, -1:0)

I think we can close the voting on this and discuss revamping the
documents to provide a way for users of IAppBuilder to migrate to the
current spec?


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-42852006
.

@serialseb
Copy link

Also, any text about the IAppBuilder should probably be a non-normative note released by the working group, as it was never part of the specification.

@panesofglass
Copy link
Member Author

I'm closing this issue. I submitted PR #24 to address the repository portions of this vote. I don't think we'll make any changes to the NuGet package except to possibly change the title to note that it will no longer be maintained (not that it needed maintenance).

@damianh
Copy link

damianh commented May 24, 2014

If pushing a new package (1.0.1?), should the interface be marked
obsolete(false)?
On 24 May 2014 16:08, "Ryan Riley" notifications@github.com wrote:

I'm closing this issue. I submitted PR #24https://github.com/owin/owin/pull/24to address the repository portions of this vote. I don't think we'll make
any changes to the NuGet package except to possibly change the title to
note that it will no longer be maintained (not that it needed maintenance).


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44090023
.

@panesofglass
Copy link
Member Author

If we push anything, we should push it as a major version so as not to break the world.

@damianh
Copy link

damianh commented May 24, 2014

What e
On 24 May 2014 16:41, "Ryan Riley" notifications@github.com wrote:

If we push anything, we should push it as a major version so as not to
break the world.


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44090918
.

@damianh
Copy link

damianh commented May 24, 2014

Whatever version, marking as obsolete would send the right message rather
than the nuget package just being delisted?
On 24 May 2014 16:41, "Ryan Riley" notifications@github.com wrote:

If we push anything, we should push it as a major version so as not to
break the world.


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44090918
.

@khellang
Copy link
Member

Is marking something obsolete (with IsError = false) considered a breaking change? I'd vote for a new version with the interface obsoleted. It'll give the people that have middleware depending on owin.dll a message to move on.

@grumpydev
Copy link

No, it's not a breaking change, that was my point, it should be false, with a meaningful message and not a major version change - that's what it's there for.

@khellang
Copy link
Member

Yeah, a simple message explaining why it's obsoleted and a link to a wiki article in this repo with a more detailed explanation and instructions on how to go forward sounds great. Also, if you were to bump major, would that be fetched with packages depending on >= 1.0.0?

@adamralph
Copy link

It would. They have to be restricted to less than 2.0 to prevent it.
On 24 May 2014 18:21, "Kristian Hellang" notifications@github.com wrote:

Yeah, a simple message explaining why it's obsoleted and a link to a wiki
article in this repo with a more detailed explanation and how to go forward
sounds great. Also, if you were to bump major, would that be fetched with
packages depending on >= 1.0.0?


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44092121
.

@damianh
Copy link

damianh commented May 24, 2014

+1 non-breaking change
On 24 May 2014 17:10, "Steven Robbins" notifications@github.com wrote:

No, it's not a breaking change, that was my point, it should be false,
with a meaningful message and not a major version change - that's what
it's there for.


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44091707
.

@skoon
Copy link
Contributor

skoon commented May 24, 2014

+1 on marking it obsolete with a big warning.

On May 24, 2014, at 10:16 AM, Damian Hickey notifications@github.com wrote:

+1 non-breaking change
On 24 May 2014 17:10, "Steven Robbins" notifications@github.com wrote:

No, it's not a breaking change, that was my point, it should be false,
with a meaningful message and not a major version change - that's what
it's there for.


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44091707
.


Reply to this email directly or view it on GitHub.

@panesofglass
Copy link
Member Author

Sounds good to me. I hadn't planned on pushing anything new to NuGet, but this recommendation sounds good. +1

@Tratcher
Copy link
Contributor

-1

  • Obsolete(Warning) is considered a breaking change since many projects enable warnings as errors.
  • Nothing should be marked obsolete unto a replacement is generally available.
  • Versioning the binary will require binding redirects in all projects.
  • Older versions of NuGet will auto roll forward to the latest patch version, but not major version, so any package version change should be a major version.

@skoon
Copy link
Contributor

skoon commented May 24, 2014

People who treat warnings as errors aren't really our concern. We just put the info out there IMO, how they react is up to them.

There is a replacement available. The func specified in the spec. :)

On May 24, 2014, at 11:04 AM, Chris R notifications@github.com wrote:

-1

Obsolete(Warning) is considered a breaking change since many projects enable warnings as errors.
Nothing should be marked obsolete unto a replacement is generally available.
Versioning the binary will require binding redirects in all projects.
Older versions of NuGet will auto roll forward to the latest patch version, but not major version, so any package version change should be a major version.

Reply to this email directly or view it on GitHub.

@markrendle
Copy link

Sorry, are we discussing a new version of OWIN.dll which will just have an Obsolete interface in it and nothing else?

Can't we just tell people?

@khellang
Copy link
Member

@markrendle That's exactly what you're doing by pushing a new version of the package. How would you tell people otherwise? The package with one interface is kinda the whole problem to start with 😉 Let's obsolete it and make it known that this is not the way moving forward 👯

@panesofglass
Copy link
Member Author

Hmm. I'm back to thinking we just empty the source and make a note on the README and in the NuGet package's description that owin.dll is done. We should make OWIN easy to adopt, not force people to jump through hoops to make things work.

My recommendations:

  • Don't push out a new NuGet package (of any version)
  • Don't delist the existing NuGet package
  • Do add text to the NuGet package description noting that owin.dll is obsolete and should not be used in new projects.
  • Do add text to the README noting that owin.dll is obsolete and should not be used in new projects.

@panesofglass
Copy link
Member Author

Okay, I've resubmitted my PR having added a note to the README.md.

@khellang
Copy link
Member

So to actually find out that owin.dll (or IAppBuilder, rather) is deprecated, you have to either drop by this repository and read that note way down in the readme or open up VS or the NuGet gallery to read the package metadata description? I understand that you might read that if you're adding the package for the first time, but what about existing middleware? Are all middleware authors involved enough for the news to reach them?

@panesofglass
Copy link
Member Author

I think most middleware authors are either directly engaged or using the Katana abstractions. I'm open to other ideas, but ultimately owin.dll us already out there. No reason to try to rewrite history. I think we should just abandon it and advise people to move onto other options.

-----Original Message-----
From: "Kristian Hellang" notifications@github.com
Sent: ‎5/‎24/‎2014 3:22 PM
To: "owin/owin" owin@noreply.github.com
Cc: "Ryan Riley" ryan.riley@panesofglass.org
Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

So to actually find out that owin.dll (or IAppBuilder, rather) is deprecated, you have to either drop by this repository and read that note way down in the readme or open up VS or the NuGet gallery to read the package metadata description? I understand that you might read that if you're adding the package for the first time, but what about existing middleware? Are all middleware authors involved enough for the news to reach them?

Reply to this email directly or view it on GitHub.

@skoon
Copy link
Contributor

skoon commented May 24, 2014

Heck I'm fairly certain all of them have posted to this issue.

On May 24, 2014, at 2:32 PM, Ryan Riley notifications@github.com wrote:

I think most middleware authors are either directly engaged or using the Katana abstractions. I'm open to other ideas, but ultimately owin.dll us already out there. No reason to try to rewrite history. I think we should just abandon it and advise people to move onto other options.

-----Original Message-----
From: "Kristian Hellang" notifications@github.com
Sent: ‎5/‎24/‎2014 3:22 PM
To: "owin/owin" owin@noreply.github.com
Cc: "Ryan Riley" ryan.riley@panesofglass.org
Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

So to actually find out that owin.dll (or IAppBuilder, rather) is deprecated, you have to either drop by this repository and read that note way down in the readme or open up VS or the NuGet gallery to read the package metadata description? I understand that you might read that if you're adding the package for the first time, but what about existing middleware? Are all middleware authors involved enough for the news to reach them?

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub.

@adamralph
Copy link

I don't see what harm it would cause to push a new version with the
obsolete attribute. It provides more channels to communicate the
information, i.e. during compilation and in the source. Is there any
downside to doing this?

@khellang
Copy link
Member

@skoon Yeah, I thought that might be the case 😝

@panesofglass I'm not saying we shouldn't abandon it. I'm saying we should get the message across as best we can 😄

@serialseb
Copy link

How about a 2.0 that has an obsolete on the current memeber and a new one that is compatible with BuildFunc? Either way, I’d say let’s delay any decision on the package until we have a BuildFunc spec and a “getting way from IAppBuilder” document ready.

Sebastien Lambla
seb@serialseb.commailto:seb@serialseb.com - http://serialseb.com

On 24 May 2014, at 23:49, Kristian Hellang <notifications@github.commailto:notifications@github.com> wrote:

Yeah, I thought that might be the case [:stuck_out_tongue_closed_eyes:] I'm not saying we shouldn't abandon it. I'm saying we should get the message across as best we can [:smile:]


Reply to this email directly or view it on GitHubhttps://github.com/owin/owin/issues/19#issuecomment-44102414.

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

No branches or pull requests