Skip to content

Upgraded Autofac to 5.1 #53

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

Merged
merged 3 commits into from
Feb 13, 2020
Merged

Upgraded Autofac to 5.1 #53

merged 3 commits into from
Feb 13, 2020

Conversation

pcherpitel
Copy link
Contributor

@pcherpitel pcherpitel commented Feb 5, 2020

Upgraded Service Fabric packages to 7.0.466
Dropped support for .Net 4.5
Upgraded package template (icon and license)

Upgraded Service Fabric packages to 7.0.464
Dropped support for .Net 4.5
Upgraded package template (icon and license)
@pcherpitel pcherpitel requested review from alexmg and tillig February 5, 2020 17:13
@pcherpitel
Copy link
Contributor Author

@alexmg @tillig the changes are pretty straight forward since it's mostly nuget packages updates let me know what you think about it and if you would like some changes before I publish the PR.

@alexmg
Copy link
Member

alexmg commented Feb 6, 2020

Looks good @pcherpitel. A few small tweaks and it's good to go!

@pcherpitel pcherpitel marked this pull request as ready for review February 6, 2020 15:11
@pcherpitel
Copy link
Contributor Author

@alexmg @tillig Can you link the PR to #52 ?

@tillig tillig linked an issue Feb 6, 2020 that may be closed by this pull request
@pcherpitel pcherpitel requested a review from alexmg February 6, 2020 21:25
@alsami
Copy link
Member

alsami commented Feb 8, 2020

Just as an info we need to finish this PR ahead of this since Autofac.ServiceFabric relies on Autofac.Extras.DynamicProxies.

@pcherpitel
Copy link
Contributor Author

Just as an info we need to finish this PR ahead of this since Autofac.ServiceFabric relies on Autofac.Extras.DynamicProxies.

From what I understand the issue in Autofac.Extras.DynamicProxies affect interface interceptors which are not used in Autofac.ServiceFabric. In this context since the changes in Autofac 5.0 are causing runtime failures if we use Autofac.ServiceFabric 2.2 can we move forward with this PR?
@tillig @alexmg @alsami

@tillig
Copy link
Member

tillig commented Feb 10, 2020

That's a good point, I don't see DynamicProxy as being referenced here. @alsami can you clarify before we move forward here?

@alsami
Copy link
Member

alsami commented Feb 10, 2020

I am not a hundred percent sure, if going forward without the fix of DynamicProxy is safe. I would wait for the fix to be merged and then update this PR accordingly. Maybe users end up using that functionality in their project anyway because it's a transitive dependency. If it's save to go on without the fix then consider my comment obsolet :)

@tillig
Copy link
Member

tillig commented Feb 10, 2020

Oh, oh, oh, I see it now. There is a reference to Autofac.Extras.DynamicProxy. The diff had it hidden from me. I agree, we need to finish that one first so we can update the reference here.

@pcherpitel
Copy link
Contributor Author

Autofac.ServiceFabric is using Autofac.Extras.DynamicProxies but only for class interceptors which are not relying on ActivatingHandlers so they are not impacted by the changes of Autofac 5.0. Autofac.Extras.DynamicProxies interface interceptors are definitely impacted by the changes of Autofac 5.0 because they rely on ActivatingHandlers and since this PR autofac/Autofac#1043 was merged back ActivatingHandlers can be called twice.
The current situation is just a bit frustrating because nothing prevent users of Autofac.ServiceFabric to update to Autofac 5.0 then they end up with runtime failure. Letting the PR go through will at least limit the impact to users that are using Autofac.Extras.DynamicProxies with interface interceptors.

@alsami
Copy link
Member

alsami commented Feb 10, 2020

The current situation is just a bit frustrating because nothing prevent users of Autofac.ServiceFabric to update to Autofac 5.0 then they end up with runtime failure.

We very much understand that this is frustrating and causes users to have problems. Which is why we need to make sure that we don't create more possible problems. We are working on the fix and as soon as that is done, we will get your PR ready and merge it 👍

@alexmg
Copy link
Member

alexmg commented Feb 12, 2020

The other PR has been unblocked now.

@pcherpitel pcherpitel changed the title Upgraded Autofac to 5.0 Upgraded Autofac to 5.1 Feb 13, 2020
Copy link
Member

@tillig tillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I'll probably increment this to 3.0.0 before publishing since Autofac 5 brings some breaking changes, but I should get it out the door shortly.

@tillig tillig merged commit ef75033 into autofac:develop Feb 13, 2020
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.

MissingMethodException after update to Autofac 5.0.0
4 participants