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

Make Auto Registration of Generic Handlers OPT-IN #1057

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

zachpainter77
Copy link
Contributor

@zachpainter77 zachpainter77 commented Aug 3, 2024

  • remove missing constraints logic.
  • remove missing constraints tests.
  • remove supporting method.
  • make feature opt in via configuration

This is a direct fix for many people's issues regarding the enforcement of constraints for generic request handers having more than two generic type parameters.

Direct fix for: #1055 , #1038

@jbogard I have made this pr so that you can decide what you want to do. Either remove the feature completely or make it Opt-In until we can implement a better resolution strategy.

- remove logic
- reomove tests
@zachpainter77 zachpainter77 changed the title Remove Missing Constraints Enforcment Remove Missing Constraints Enforcment On Generic Handlers Aug 3, 2024
@crnd
Copy link

crnd commented Aug 4, 2024

Is this PR now just removing the protections from the registration process to make errors go away? Won't this now potentially cause users to end up having a lot of unwanted service registrations and potentially timeouts happening during application startup as the feature has been implemented opt-out instead of opt-in?

In my opinion the whole feature should either be removed or then the feature should be made opt-in to make sure that no one ends up getting this enabled by default by just upgrading to a newer minor version. As already stated before, this is anyway too late at this point as 12.3.0 and newer versions have already been released, but the current approach should have been a major version increment.

@zachpainter77
Copy link
Contributor Author

zachpainter77 commented Aug 4, 2024 via email

@zachpainter77
Copy link
Contributor Author

Is this PR now just removing the protections from the registration process to make errors go away? Won't this now potentially cause users to end up having a lot of unwanted service registrations and potentially timeouts happening during application startup as the feature has been implemented opt-out instead of opt-in?

In my opinion the whole feature should either be removed or then the feature should be made opt-in to make sure that no one ends up getting this enabled by default by just upgrading to a newer minor version. As already stated before, this is anyway too late at this point as 12.3.0 and newer versions have already been released, but the current approach should have been a major version increment.

Ok, I have made the feature Opt-In.

image

@zachpainter77 zachpainter77 changed the title Remove Missing Constraints Enforcment On Generic Handlers Make Auto Registration of Generic Handlers OPT-IN Aug 4, 2024
@zachpainter77
Copy link
Contributor Author

@crnd

I have made another PR alongside this one that will completely remove the feature. So now it is only up to @jbogard to decide what PR to merge.

  • cheers

@nhwilly
Copy link

nhwilly commented Aug 12, 2024

This bit me pretty hard when I added MassTransit to my app.

@alexkuznetsov
Copy link

@zachpainter77 @jbogard

Hi, any chance of adding this and releasing the nuget package?

After upgrading to version 12.4, we had a few interesting hours to investigate what the ... going on,
and after some googling and reading issues on github we found a solution using
the RegisterGenericHandlers configuration property.

@nhwilly
Copy link

nhwilly commented Sep 9, 2024

Same here. Installed MassTransit and promptly lost a day tracking it. Backed down a version to fix it.

@jbogard
Copy link
Owner

jbogard commented Sep 9, 2024

Ugh so I think I'll push a version with this behavior opt-in but I want to tackle this much differently. Manually registering and scanning isn't really feasible.

@jbogard jbogard merged commit fb30902 into jbogard:master Sep 9, 2024
4 checks passed
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