Breaking changes in the new release #152
Comments
|
Sadly, it's easier said than done. I've considered and wanted to do something obvious like throw an exception when Worse still, even if I could figure out how and where to throw an appropriate exception, that wouldn't solve everything (although arguably the most common issues). Since In the interim, the best solution I have so far was to update the documentation everywhere. The release notes have called out the changes since the RC. All of the wiki and code samples have been updated. I even updated the landing page to remind people to read the release notes. Unfortunately, I can't make people read the content.
|
|
I understand and fortunately I at the end found the release notes of the Asp Net Versioning. Honestly I read the release notes for the 1.1 version and I have not seen this information about breaking changes (see here, here, here,.... On another note, I wanted to know: is there a possiblity to debug the 404 error in the Aspnet core application? I often find myself commenting/uncommenting code just for understanding why I get a 404 code because there is no debug logs. I tried the Aspnet core Diagnostics Elm but it throws exceptions + it does not seem to have more information than the ones displayed in the console logs. |
|
In general, I don't know of any great ways to troubleshoot 404. API Versioning in ASP.NET Core 1.1+ exacerbates the problem when things are not configured properly because the IActionSelector intentionally indicates that it has not found a match, even if it has. This is a limitation in the way that ASP.NET Core works and is currently the only way to ensure that all possible action candidates have been considered. It's definitely something worth investing in. If you have some specific ideas or wants, it might be worth spinning up a new issue titled something along the lines of "Improve Logging and Diagnostic Information" with your requirements and/or ideas. |
|
@ales004, @idevtech, @Alezis, @irowbin, @exiled78, @YogirajA, @Eneuman, @matteobortolazzo, @ericvan76, @TonyCasey If I've called you out in the list, it's because you have recently filed an issue or otherwise commented on issues relating the upgrade for API versioning in ASP.NET Core from 1.0 to 1.1; specifically, related to the new requirement configuration to call I never like this new addition and up until now, I haven't been able to find any alternatives. I let the RC bake in for a while and the change didn't seem to hang up the early adopters much, so I felt comfortable going forward with the stable release. It's clear now that, this new requirement is hanging up a lot of people. Despite all the documentation and issues, I suspect more issues will keep coming in. I'm hearing the feedback and I've been scrambling for a better approach. I'm happy to report that I believe I've found an alternative that gets rid of the need to have I've published 1.2.0-beta1 of the package that includes the update. If at least a few of you can verify that change doesn't break you (obviously, minus the removal of If three or more of you can take a few minutes to verify, I think that puts everyone in a good state and we can remove this snare for good. Thanks for your help and your voices that this needed to be improved. |
|
I would suggest clean 'app.UseApiVersioning' and mark it as obsoleted, and
remove it in a later version. It's a bit annoying adding and removing
things so quickly. Most of people won't check the document when they
upgrade a package. Anyway I will test new version and let you know later
today. :)
…On Fri, 30 Jun 2017 at 4:10 am, Chris Martinez ***@***.***> wrote:
@ales004 <https://github.com/ales004>, @idevtech
<https://github.com/idevtech>, @Alezis <https://github.com/alezis>,
@irowbin <https://github.com/irowbin>, @exiled78
<https://github.com/exiled78>, @YogirajA <https://github.com/yogiraja>,
@Eneuman <https://github.com/eneuman>, @matteobortolazzo
<https://github.com/matteobortolazzo>, @ericvan76
<https://github.com/ericvan76>, @TonyCasey <https://github.com/tonycasey>
If I've called you out in the list, it's because you have recently filed
an issue or otherwise commented on issues relating the upgrade for API
versioning in ASP.NET Core from 1.0 to 1.1; specifically, related to the
new requirement configuration to call app.UseApiVersioning.
I never like this new addition and up until now, I haven't been able to
find any alternatives. I let the RC bake in for a while and the change
didn't seem to hang up the early adopters much, so I felt comfortable going
forward with the stable release.
It's clear now that, this new requirement is hanging up a lot of people.
Despite all the documentation and issues, I suspect more issues will keep
coming in. I'm hearing the feedback and I've been scrambling for a better
approach. I'm happy to report that I believe I've found an alternative that
gets rid of the need to have app.UseApiVersioning at all. I was able to
run through all the test cases and some repros that you have provided.
Everything still works. :)
I've published 1.2.0-beta1
<https://www.nuget.org/packages/Microsoft.AspNetCore.Mvc.Versioning/1.2.0-beta1>
of the package that includes the update. If at least a few of you can
verify that change doesn't break you (obviously, minus the removal of
app.UseApiVersioning), then I feel good about promoting the package to
the stable 1.2.0 release. This would also have the added benefit of
removing knowledge about the order in which things must be registered (e.g.
app.UseMvc, then app.UseApiVersioning, then app.MapWhen, etc, etc). It
also appears that the majority of devs are still using 1.0.x based on the
download numbers. Should things pan out. I'm considering delisting the
1.1.x packages altogether so people stop running into these landmines.
If three or more of you can take a few minutes to verify, I think that
puts everyone in a good state and we can remove this snare for good. Thanks
for your help and your voices that this needed to be improved.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJCzmtVN7WeMP-XqrTx_0qooERk-vaeks5sI--5gaJpZM4OIFbL>
.
|
|
I hear you. I hate flip-flopping. I considered just making it obsolete. It's not off the table - yet. That was always the eventual plan. Not sure if anyone else has an opinion on the matter. I could equally just leave the method in place and have it do nothing. Thanks for help with the verification. |
|
1.2.0-beta1 works well here. |
|
Hi Chris
Tested 1.2.0-beta1 and it appears to work correctly.
Chris
…On Thu, Jun 29, 2017 at 2:40 PM, Chris Martinez ***@***.***> wrote:
@ales004 <https://github.com/ales004>, @idevtech
<https://github.com/idevtech>, @Alezis <https://github.com/alezis>,
@irowbin <https://github.com/irowbin>, @exiled78
<https://github.com/exiled78>, @YogirajA <https://github.com/yogiraja>,
@Eneuman <https://github.com/eneuman>, @matteobortolazzo
<https://github.com/matteobortolazzo>, @ericvan76
<https://github.com/ericvan76>, @TonyCasey <https://github.com/tonycasey>
If I've called you out in the list, it's because you have recently filed
an issue or otherwise commented on issues relating the upgrade for API
versioning in ASP.NET Core from 1.0 to 1.1; specifically, related to the
new requirement configuration to call app.UseApiVersioning.
I never like this new addition and up until now, I haven't been able to
find any alternatives. I let the RC bake in for a while and the change
didn't seem to hang up the early adopters much, so I felt comfortable going
forward with the stable release.
It's clear now that, this new requirement is hanging up a lot of people.
Despite all the documentation and issues, I suspect more issues will keep
coming in. I'm hearing the feedback and I've been scrambling for a better
approach. I'm happy to report that I believe I've found an alternative that
gets rid of the need to have app.UseApiVersioning at all. I was able to
run through all the test cases and some repros that you have provided.
Everything still works. :)
I've published 1.2.0-beta1
<https://www.nuget.org/packages/Microsoft.AspNetCore.Mvc.Versioning/1.2.0-beta1>
of the package that includes the update. If at least a few of you can
verify that change doesn't break you (obviously, minus the removal of
app.UseApiVersioning), then I feel good about promoting the package to
the stable 1.2.0 release. This would also have the added benefit of
removing knowledge about the order in which things must be registered (e.g.
app.UseMvc, then app.UseApiVersioning, then app.MapWhen, etc, etc). It
also appears that the majority of devs are still using 1.0.x based on the
download numbers. Should things pan out. I'm considering delisting the
1.1.x packages altogether so people stop running into these landmines.
If three or more of you can take a few minutes to verify, I think that
puts everyone in a good state and we can remove this snare for good. Thanks
for your help and your voices that this needed to be improved.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdqcgaJRdgdjVuVAXvT70_o9C4TjZVFks5sI--5gaJpZM4OIFbL>
.
|
|
Great! Thanks for confirming. I think we're about good. A final question to the community. There are a couple of ways that Option 1Just delete it. This is the case in the 1.2 beta. I agree it's lame to add it, only to subsequently remove it so quickly. The only positive note is that adoption for 1.1 is only at about %1 of 1.0.x. Given the low adoption and how much trouble it's caused people, I'm ready to ditch it all together. The need to remove it is obvious, provides better continuity with 1.0.x, and has a relatively low negative impact. Option 2Another option is to keep The future direction will definitely be to:
Thoughts? Votes? Thanks. |
|
My vote is Option 1 and remove the 1.1 build. Why keep something around
that could cause other people problems.
Chris
…On Fri, Jun 30, 2017 at 9:07 PM, Chris Martinez ***@***.***> wrote:
Great! Thanks for confirming. I think we're about good.
A final question to the community. There are a couple of ways that
app.UseApiVersioning can be sunset.
Option 1
Just delete it. This is the case in the 1.2 *beta*. I agree it's lame to
add it, only to subsequently remove it so quickly. The only positive note
is that adoption for 1.1 is only at about %1 of 1.0.x. Given the low
adoption and how much trouble it's caused people, I'm ready to ditch it all
together. The need to remove it is obvious, provides better continuity with
1.0.x, and has a relatively low negative impact.
Option 2
Another option is to keep app.UseApiVersioning, but turn it into a no-op
method. I would also hide this method from Intellisense. I'm debating
whether adding [Obsolete] is worth it. Personally, I don't like warnings
in my code, so if I saw a warning that it's obsolete, then I'd remove the
code altogether. If that's the general behavior, then why bother supporting
that for 1% of users? I might as well go with Option 1 in that case. If I
don't mark app.UseApiVersioning as [Obsolete], you won't get any ugly
error messages and it won't do anything. That would be the smoothest,
quietest transition for folks that went to 1.1, but are now moving to 1.2.
People that move straight to 1.2 will never notice the difference.
------------------------------
The future direction will definitely be to:
- Remove app.UseApiVersioning in 2.0, which isn't all the far off
- Optionally, unlist the 1.1 package; it's caused enough pain that for
those who haven't gone to 1.1 yet,
I'd rather just have them go straight to 1.2.
Thoughts? Votes? Thanks.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdqckK1yYAIa36Qblbk-NeuelJm71elks5sJZu5gaJpZM4OIFbL>
.
|
|
@commonsensesoftware Thanks for the update! I tested the beta package and it worked on my machine 👍 I didn't test it in our test environments. I'd like to vote for option1 since there was such low adoption of the new package. Can you make the one with |
|
I recommend unlisting the |
|
Same here - it has been two days of research until I found out that it was the update of this nuget package that caused the 404 errors in my net core mvc app. Fortunately I have found this thread here, that helped me fixing it by adding |
|
Thanks for all the feedback, suggestions, and verifications. Here's the final course of actions:
|
After upgrading to 1.1 404 is returned to all webapi requests.
After upgrading to .Net Core 1.1, it took me one day of work to understand that my WebApi was returning 404 to all requests because I did not added: app.UseApiVersioning(); in the Startup class.
Maybe you should throw an exception when AddApiVersioning is used but UseApiVersioning is not? Returning 404 does not seem to be the answer.....
The text was updated successfully, but these errors were encountered: