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

[WIP] DotNed standard platform #51

Closed
wants to merge 12 commits into from

Conversation

mauricedb
Copy link
Contributor

Hi Michael,

As part of a hackaton day at the MVP Summit I converted Polly to use the new .NET generations that will soon be released wen ASP.NET 5 is coming. See here for more details. With this Polly supports CoreCLR and can be used with the new ASP.NET 5 projects.

I would not merge this right now. It definitely needs more work before it can be used. For one it is still referring to bunch of beta assemblies. Also I didn't remove all old projects that are no longer needed or update the nuspec file.

More interestingly. When I run the unit tests using the full CLR, either the .NET 4.5 or PCL project, everything tests just fine. However when I test using the new CoreCLR test project I get random tests failing. I didn't really investigate but I did notice that SystemClock.Sleep is a static and that could be an issue with parallel tests using XUnit.

Another things I haven't done yet is run this on Linux or on Mac.

Hopefully this will prove to be a useful start for using Polly on CoreCLR after the release though.

Should_reset_circuit_after_the_specified_duration_has_passed_if_the_next_call_does_not_raise_an_exception
Should_close_circuit_after_the_specified_duration_has_passed
hould_sleep_for_the_specified_duration_each_retry_when_specified_exception_thrown_less_number_of_times_then_there_are_sleep_durations
@michael-wolfenden
Copy link
Contributor

Hey @mauricedb,

Thanks once again for you help.

I restructured the project recently in an attempt to add .NET Core support following https://oren.codes/2015/07/29/targeting-net-core/. I haven't released the updated nupkg's yet as I haven't done extensive enoguh testing yet.
I looked at moving to the new project.json system, but at the time my understanding was that I could not target 3.5 or the other PCL plateforms. I'm not sure if this is still the case and in either case I figured if I could get away with providing .NET Core support using the existing system I would stick to that.

In terms of testing, your right. SystemClock.Sleep being static was a bad idea. I bit me hard when I moved to xunit2 which runs all the tests in parallel. I was getting random test failures and ended up having to disable running the tests in parallel

See:
https://github.com/michael-wolfenden/Polly/blob/master/src/Polly.Pcl.Specs/Properties/AssemblyInfo.cs#L5
https://github.com/michael-wolfenden/Polly/blob/master/src/Polly.Net45.Specs/Properties/AssemblyInfo.cs#L5

I have to admit, I'm finding all the changes to the .NET platform confusing and your advice would be appreciated. Are the changes I've already made enough to run Polly on .NET Core? If not can I move to the new project format and still support the same target?

@mauricedb
Copy link
Contributor Author

Hi @michael-wolfenden,

I agree all the changes are confusing and I certainly don't completely
understand then yet. However last week I had the opportunity to work on
this during the MVP Summit with guidance MS engineers who actually work on
the core bits as well as yet to be published documentation and that helped
a lot.

The first step they advise is to switch from the old project files to the
new class library web project. With the old system each project can only
target a single environment like .NET45, .NET 4 or PCL. With the new
project.json you can target multiple environments from a single project. So
I turned the Polly.Shared project into a class library targeting the 4
previous frameworks and added dnxcore50. This lets us delete the other
runtime projects and build with DNU build. The result is

image

Single project with multiple outputs :-)

I tried to do the same with the specs but was less successful there. The
.NET framework and CoreCLR is fine but with the PCL version of the specs I
have been unable to get this to work so far. Basically FLuentAssertions and
Polly target a different PCL profile and have not been able to get that to
work yet. If that works we should have a single Spec project. With dnx
test
I can run the coreclr specs. Haven't looked into running the net45
ones yet but that should not be hard. Don't know about running then with R#
or nCrunch yet though. This is all still very much beta so I think tooling,
even VS2015 in some parts, still needs to catch up.

One thing I don't really get yet is the targeting of dotnet versus
dnxcore50. I understand we should target dotnet but the whole
standard-platform and the versions mentioned is not available yet AFAIK so
we can't use that yet. The whole point of this is to target API versions
instead of different platforms as the number of platforms just keeps on
growing. By specifying the API version you indicate what platforms you
actually support
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/standard-platform.md#mapping-the-net-platform-standard-to-platforms.
So the Profile259 should become netstandard1.0 eventually.

There where some more people working on CsvHelper last week, that PR
JoshClose/CsvHelper#445 might also help shed some
light.

HTH
Maurice

@joelhulen
Copy link
Member

@mauricedb I am starting to work toward having Polly support .NET Core. Are you interested in helping out? Things have matured with the Core framework lately, which will help ensure there are no more breaking changes.

Microsoft recommends that all libraries, such as Polly, convert to .NET Core sooner rather than later. That way, once people start building solutions in Core, or converting existing projects to it, libraries will already be taken care of.

@mauricedb
Copy link
Contributor Author

@joelhulen I would love to help out. Unfortunately I am completely saturated with work right now though so I won't be able to do much for at least the next month or so. As far as maturity goes I am not sure how much churn the whole renaming to 'dotned' will cause but I suspect a fair bit.

@shmuelie
Copy link

shmuelie commented Jun 16, 2016

I have a working branch on my local clone. Passes all but 45 tests!

I would note that this is a "must" for me as I'm working on a Core project for production that badly needs polly

@reisenberger
Copy link
Member

@SamuelEnglard Do you want to push that local clone up to your remote, so that we can view? (can't find on your remote).

Dilemma is getting sucked into chasing a moving target with .NET Core/Standard while the tooling is still in such a state of flux, and that spannering work for weeks on the other features on the Polly Roadmap. Maybe (hopefully 🙏 ) in only just couple weeks Microsoft will come out w/ a clear direction and more settled version of tooling. OTOH, we appreciate ppl want to get on w Polly in .NET core/standard.

@SamuelEnglard Certainly happy to look at what you've done and see if we can find quick win? I can 👓 over the tests.

@shmuelie
Copy link

@reisenberger honestly not sure why I didn't push it up but did now! https://github.com/SamuelEnglard/Polly/tree/netcore/

@reisenberger
Copy link
Member

@reisenberger should I make a proper pull request?

@SamuelEnglard Hang fire on a new PR just now (tho thanks). I am hoping to release Polly 4.3.0 (delivering #14) in the coming week; and we may get some new/changed tooling from Microsoft over next few days too.

@reisenberger
Copy link
Member

reisenberger commented Jul 1, 2016

@SamuelEnglard I expect your test failures are around tests which manipulate properties on SystemClock? Because this is (currently) a static in the Polly design, tests cannot be run in parallel. Add [assembly: CollectionBehavior(DisableTestParallelization = true)] to the relevant AssemblyInfo.cs and you should clear the test failures.

@SamuelEnglard @mattwoberts I am currently preparing a Polly pre-release nuget package supporting .NET Core (following your start @SamuelEnglard; thanks @SamuelEnglard !). Good progress. Seems good in VS. Just working @joelhulen on getting the cake build to run the new .NET core tests, so that we can have a fully mature CI process around this. @SamuelEnglard @mattwoberts it would be great if you could test the nuget package in (hopefully) next few days.

.NET Core RTM 1.0 from Microsoft Mon27 didn't add any new tooling, so this is all still project.json-based - which of course is deprecated and will (later) go away.

@shmuelie
Copy link

shmuelie commented Jul 1, 2016

@reisenberger Yup, that attribute fixed it!

@reisenberger One thing to note: I had to make some changes to the packages I reference to work in RTM but otherwise all good :)

@reisenberger
Copy link
Member

Closed in favour of #132. However, thanks to @mauricedb for his early work on this, shining the light on many of the issues which are still relevant as .NET Core goes RTM.

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

Successfully merging this pull request may close these issues.

5 participants