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

Not able to find method enableAutoConfig in GuiceBundle in version >= 1.0.0.0 #41

Open
harmishlakhani opened this issue Sep 10, 2018 · 5 comments

Comments

@harmishlakhani
Copy link

In dropwizard-guice we have enableAutoCongig method present in which we can give base package and Dropwizard and Guice will take for resource classes.
In dropwizard-guicer we are not able to find such method and because of that we have to tell jersey for base package.
Because of this Guice is not able to inject dependencies of services into resources.
So I think if somehow we are enabling this package scan phase in guicer then we should be able to inject service as well with use of @Inject annotation to resources.

@jhaber
Copy link
Member

jhaber commented Sep 13, 2018

We ultimately found the AutoConfig feature in dropwizard-guice to be harmful for a few reasons. First, we found the reflections library to be unreliable; it wouldn't always discover everything that it should, and this caused people to not trust the feature. Second, it was too magical and hard to reason about. Seemingly harmless code reorganization would break applications because AutoConfig was relying on a specific package structure. Third, it was hard to override. If something gets picked up by AutoConfig that you don't want, there is no natural way to stop it from happening.

In dropwizard-guicier, you would bind these classes in Guice and they would get added to the Dropwizard environment. There is an example of this here:
https://github.com/jhaber/dropwizard-guicier-example/blob/7d5e5161b4dddfe99acbcf99f72bfd8a16540e5f/src/main/java/com/hubspot/dropwizard/example/ExampleModule.java#L20-L38

This may require a little more code, but we have found that this extra step makes things much more explicit, reliable, and easy to reason about.

We don't have plans to re-introduce functionality similar to AutoConfig, but you could probably recreate it using something like this: https://github.com/manzke/guice-automatic-injection which I think you could use to create Guice bindings via classpath scanning. I'd also be curious if you have any use-cases that require AutoConfig, or if it's just a matter of convenience.

@liorhar
Copy link

liorhar commented Sep 13, 2018

Thanks for the explanation, I can see why some teams don't like classpath scanning. But, :-) ... there are teams that are used to it, especially those coming from the Spring framework. Since it was an optional configuration with a default set to false I don't see why wouldn't the library support it and let the programmer decide if he likes auto config or not. You're right it's easy to add, I did it, but I also noticed there were at least 3 issues about this lost functionality. So I guess people use it. To simplify the migration from the previous version I think it would be nice to add it.
Are you open for a pull-request to address this?

@jhaber
Copy link
Member

jhaber commented Sep 14, 2018

I don't think we should add native support in dropwizard-guicier, and instead rely on users who want that functionality to add it themselves. I don't imagine this being particularly onerous, and it also seems like this functionality could be bundled up into a separate library.

@jbridger
Copy link

I could use some guidance on registering an implementation of ConfiguredBundle while also performing dependency injection. What is the correct way to do it given AutoConfig would have picked it up previously?

@jhaber
Copy link
Member

jhaber commented Sep 17, 2019

Do you have a code sample? In particular, what does your ConfiguredBundle do?

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

No branches or pull requests

4 participants