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

AspNet.Core support #5

Closed
LeonidEfremov opened this issue Jul 27, 2017 · 22 comments
Closed

AspNet.Core support #5

LeonidEfremov opened this issue Jul 27, 2017 · 22 comments
Assignees
Projects

Comments

@LeonidEfremov
Copy link

Any plans for migration?

@bronumski
Copy link
Owner

@LeonidEfremov - Sorry I never got notified of the issue creation.

I am currently in the middle of creating a package for AspNet Core right now. I'll give you a shout when I have something. Currently hitting an issue with nuget to package the projects up at the moment.

@LeonidEfremov
Copy link
Author

@bronumski, thank you =)

@LeonidEfremov
Copy link
Author

Hi, @bronumski! Any progress on this issue? We really need this update =)

bronumski added a commit that referenced this issue Mar 3, 2018
bronumski added a commit that referenced this issue Mar 3, 2018
bronumski added a commit that referenced this issue Mar 3, 2018
bronumski added a commit that referenced this issue Mar 3, 2018
bronumski added a commit that referenced this issue Mar 4, 2018
@bronumski
Copy link
Owner

@LeonidEfremov - Just pushed some changes. I have not tested the packages yet. Appveyor picked up the change and published them before I could configure it to push out a beta release. I'll test it later and if it doesn't work I will remove the package.

@bronumski
Copy link
Owner

bronumski commented Mar 9, 2018

@LeonidEfremov - I backed out the package and pushed up a beta version

Install-Package HealthNet.AspNetCore -Version 1.2.65-dotnet-core-beta

@bronumski bronumski self-assigned this Mar 9, 2018
@bronumski bronumski added this to To do in HealthNet via automation Mar 9, 2018
@bronumski bronumski moved this from To do to In progress in HealthNet Mar 9, 2018
@LeonidEfremov
Copy link
Author

Wow, great! Will be test it next 2-3 days, and report to you.

@LeonidEfremov
Copy link
Author

LeonidEfremov commented Mar 10, 2018

Can you change signature for AddHealthNet extension to be core-style, I mean add action:

IServiceCollection AddHealthNet(Action<HealthNetConfiguration> setupAction)

@LeonidEfremov
Copy link
Author

LeonidEfremov commented Mar 10, 2018

We still need to register each ISystemChecker manualy:

services.AddTransient<ISystemChecker, CustomChecker>();

think better implementation will be AutoMapper-style:

services.AddAutoMapper(GetType().Assembly)

@LeonidEfremov
Copy link
Author

Everything working well =) Just one warning

warning NU1603: HealthNet.AspNetCore 1.2.69-dotnet-core-beta depends on H
ealthNet.Core (>= 1.2.0) but HealthNet.Core 1.2.0 was not found. An approximate best match of HealthNet.Core 1.2.61 was resolved.

@bronumski
Copy link
Owner

Yes I got the warning too. I need to change how it is packaged.
There are a number of tweaks that I would like to make and all your feedback is useful. I'll make some additional stories for them. I subscribe to lean so it's better to get something out there first which can be used and use feedback to add additional functionality/tweaks.

This would be a good one.

IServiceCollection AddHealthNet(Action<HealthNetConfiguration> setupAction)

I'm not sure about this though:

services.AddAutoMapper(GetType().Assembly)

Not that I don't like auto wire up, I do. I want to get a better idea of how teams are using core IoC and if they are using anything on top such as Windsor, AutoFac etc which would already do the above. So not dismissing it, but I want to look around at other projects and see what they are doing.

@bronumski
Copy link
Owner

What are your thoughts on this instead of the above.

  public void ConfigureServices(IServiceCollection services)
  {
    ...
    // Automatic wire up of all implementations of ISystemChecker in assembly containing config
    services.AddHealthNet<CustomHealthCheckConfiguration>();
    // If assembly contains an implementation of IVersionProvider that will be used instead of
    // the built in AssemblyFileVersionProvider

    // Manual wire up
    services.AddTransient<IHealthNetConfiguration, CustomHealthCheckConfiguration>();
    services.AddTransient<ISystemChecker, FooSystemChecker>();
    services.AddHealthNet();
    ...
  }

  public void Configure(IApplicationBuilder app)
  {
    ...
    app.UseHealthNet();
    ...
  }

I would be interested in seeing a use case for the Action approach IServiceCollection AddHealthNet(Action<HealthNetConfiguration> setupAction). Maybe the two approaches can be combined.

I am going to push the above if you want to try it out.

@LeonidEfremov
Copy link
Author

Will test it today

@LeonidEfremov
Copy link
Author

LeonidEfremov commented Mar 15, 2018

So, version 1.2.71-dotnet-core-beta don't working for me

System.IO.FileNotFoundException: 'Could not load file or assembly 'HealthNet.Core, Version=1.2.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.'

while application starting. Also this interface looks not well:

IServiceCollection AddHealthNet<THealthNetConfig>(this IServiceCollection service)

think better is

IServiceCollection AddHealthNet(Action<HealthNetConfiguration> setupAction)

@bronumski
Copy link
Owner

Ok I will check it out. I have not had time to fully test the package, just the run the intregration tests.

Can you do an example of how you would expect a usage of your above method might look?

@bronumski
Copy link
Owner

bronumski commented Mar 16, 2018

The version error that you are getting is because of the way I generate the nuspec files and they don't know about the dependency version suffix, they only know the version number. Therefore the dependency version is put in as 1.2.71 instead of 1.2.71-dotnet-core-beta.

To get around this make sure you install both packages:

dotnet add package HealthNet.Core -v 1.2.71-dotnet-core-beta
dotnet add package HealthNet.Core.AspnetCore -v 1.2.71-dotnet-core-beta

By default .net core projects have <WarningsAsErrors>NU1605</WarningsAsErrors> on by default. To get round this disable it in your project file by adding <NoWarn>NU1605</NoWarn>.

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
    <NoWarn>NU1605</NoWarn>
  </PropertyGroup>

This will all be sorted when it is published as a non beta package. Right now I don't have time to change the way the packages are built.

I can confirm that I have done this to a new project and it works as expected. Just waiting on your feedback about the extension methods.

@LeonidEfremov
Copy link
Author

LeonidEfremov commented Mar 16, 2018

Thx, i start application and everything is working well.
This is how i expect to use HeathNet.

  1. Register in ApplicationBuilder
 app.UseHealthNet();
  1. Add Service
 services
    .AddTransient<ISystemChecker, DefaultChecker>()
    .AddHealthNet(HeathConfiguration);
  1. Overridable Initialization
protected virtual void HeathConfiguration(IHealthNetConfiguration configuration)
{ 
    configuration.Path = "health";
    configuration.DefaultSystemCheckTimeout= TimeSpan.FromSeconds(10);
}

@bronumski
Copy link
Owner

So in 3 you don't want to use a custom configuration object. What you lose here is that, if you use a custom configuration object the service will:

  1. Pull the file version from the assembly containing your configuration object.
    2, It will automatically wire up any objects implementing ISystemChecker.
  2. It will also pull out the first object it finds that implements IVersionProvider and wire that up if found overriding the provider used for 1 above.

This means your add service in step 2. would just be:

services
    .AddHealthNet<CustomHealthcheckConfig>();

Perhaps we can have an override of that allows the above but currently HealthNetConfiguration is abstract. Making it not abstract would have bigger implications on how the service itself starts up. This might have to be dealt with at a later date.

@LeonidEfremov
Copy link
Author

This way is ok to, but i prefer Action instead of generic.

@LeonidEfremov
Copy link
Author

When do you plan to release?

@bronumski
Copy link
Owner

I was considering switching to async calls so that we could have it async all the way from the controller / module but I'll leave that for another time. I'll try and push it shortly.

@LeonidEfremov
Copy link
Author

ping =)

bronumski added a commit that referenced this issue Apr 13, 2018
bronumski added a commit that referenced this issue Apr 13, 2018
bronumski added a commit that referenced this issue Apr 13, 2018
@bronumski
Copy link
Owner

Merged and released.

https://www.nuget.org/packages/HealthNet.Core/1.2.74

HealthNet automation moved this from In progress to Done Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
HealthNet
  
Done
Development

No branches or pull requests

2 participants