Travix-International / Hystrix.Dotnet

A combination of circuit breaker and timeout. The .net version of the open source Hystrix library built by Netflix.
https://travix-international.github.io/Hystrix.Dotnet/
MIT License
95 stars 16 forks source link

Add the Hystrix stream implementation for OWIN #22

Closed markvincze closed 4 months ago

markvincze commented 6 years ago

Add a new Stream implementation using OWIN, and extract the Configuration handling to a separate project so that it doesn't have to be duplicated.

markvincze commented 6 years ago

Thanks for issuing the PR, @klettier! I took a quick look and looks good to me, I'll try to review it properly this week.

klettier commented 6 years ago

Thanks, I would like to change some little things like the order of the parameters in the extensions (in order to be consistent) and add the Owin sample project and tests with owin testing, do you think you could wait a bit?

markvincze commented 6 years ago

@klettier, yeah, sure, no problem ;) Just ping when you're done.

klettier commented 6 years ago

Changelog :

!! Breaking change !!

Note that due to the move of WebConfiguration into his proper project (Hystrix.Dotnet.WebConfiguration) the namespece of the object HystrixConfigSection have changed and must be modified accordingly in all config file.

PS: I will travel during 2 weeks from today so I couldn't contribute more during this period.

klettier commented 6 years ago

Any feedback on this?

klettier commented 6 years ago

I'm back, any news?

markvincze commented 6 years ago

Please add the commands for building and testing the new Owin project to the appveyor.yml.

markvincze commented 6 years ago

Please adjust the namespace of HystrixConfigSection according to the breaking change everywhere we use it:

I think that's all the places.

markvincze commented 6 years ago

The last thing is bumping the version numbers. To make our lives easier we usually bump the version numbers of all the project released to Nuget (and keep them the same). So you'll have to set the version number (in the VersionPrefix property) on the projects Hystrix.Dotnet, Hystrix.Dotnet.AspNet, Hystrix.Dotnet.AspNetCore, and the new Hystrix.Dotnet.Owin.

@JorritSalverda what do you think, do we have to bump to 2.0, since the namespace change of the config section class is a breaking one, or is it enough to just make it 1.1.0?

JorritSalverda commented 6 years ago

It's a breaking change, so we should definitely bump the major version. Otherwise we run the risk of breaking tons of Travix applications.

klettier commented 6 years ago

Hello,

Thank you for your feedback, I applied the changes accordingly and centralized common information in a props file for easy management.

I think It would be wise to provide packages in beta (2.0.0-beta1) and schedule the release in january ?

Regards,

kévin

JorritSalverda commented 4 months ago

Hey @klettier I still see this open PR, but I'm no longer maintainer of this repo. Can you remove me as reviewer of close the PR? Thanks.

klettier commented 4 months ago

@JorritSalverda I have no power here foo