Skip to content

Initialize Guice after the environment has been set#76

Closed
chrisrun wants to merge 4 commits into
HubSpot:masterfrom
chrisrun:master
Closed

Initialize Guice after the environment has been set#76
chrisrun wants to merge 4 commits into
HubSpot:masterfrom
chrisrun:master

Conversation

@chrisrun
Copy link
Copy Markdown

This is a simple way to achieve a lazier initialization of the guice injector. There are no tests that proof the singletons are working correctly. If you are willing to accept the pull request I might add some:).

@scho
Copy link
Copy Markdown

scho commented Nov 24, 2015

Does that mean, that we don't need to build our Guice bundle with Stage.DEVELOPMENT anymore?
That would be awesome!
I don't know much about Guice internals and the details about your implementation. So I can't really judge if your solution is the proper way to do it or not.

Edit:
Maybe you can re-use the tests from #70 in order to verify that your solution works?

@chrisrun
Copy link
Copy Markdown
Author

The change is very simple and its only about the time when guice is being initialized. This works for singletons that need the environment, so far nothing broke, but I haven't tested anything else. If I find a moment Ill look into tests from #70.

@jhaber
Copy link
Copy Markdown
Member

jhaber commented Nov 24, 2015

This almost works but unfortunately breaks an important feature of dropwizard-guice that many people are depending on. You can't just move the call to initialize into the run phase like this because it is too late to add bundles or configured bundles to the bootstrap by that point. I added a test in 4d079c9 that shows the issue (the bundle will never be run).

Over time, however, we've found that auto config actually does more harm than good and we've moved away from it towards a more explicit model (auto config makes it hard/impossible to do conditional binding like this). We have another library, HubSpot/dropwizard-guicier, that doesn't support auto config. It builds the injector during the run phase, runs in Stage.PRODUCTION by default, and has many other benefits. It does however still use dropwizard 0.7 and Jersey 1 at the moment, but I might try to upgrade it soon (boo HK2)

@chrisrun
Copy link
Copy Markdown
Author

I see... we could not use autoconfig for another reason and maybe for this reason it did not give problems.

@jhaber
Copy link
Copy Markdown
Member

jhaber commented Aug 6, 2016

Unfortunately I don't think we'll be able to fix this issue in dropwizard-guice, but I'd recommend checking out HubSpot/dropwizard-guicier

@jhaber jhaber closed this Aug 6, 2016
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.

3 participants