mekomsolutions / openmrs-module-initializer

The OpenMRS Initializer module is an API-only module that processes the content of the configuration folder when it is found inside OpenMRS' application data directory.
MIT License
23 stars 79 forks source link

[#211] Undo InitializerDAO XML injection in favour of autowiring injection. #228

Closed Ruhanga closed 1 year ago

Ruhanga commented 1 year ago

This is a followup PR to address a runtime bug caused by the InitializerDAO not being set. Issue: https://github.com/mekomsolutions/openmrs-module-initializer/issues/211

mseaton commented 1 year ago

@Ruhanga / @mks-d . This was already merged, so not sure you need or want my review still. No reason to oppose this, as long as it works and doesn't lead to performance issues, particularly at startup. I would like to know why this is necessary though - do we know what kind of situation works when we inject dependencies via autowiring and does not work when we explicitly inject using xml mappings? Seems odd.

mogoodrich commented 1 year ago

@Ruhanga @mks-d I confirmed that this change (Autowired in a service) literally added 4 minutes to the start up time of our PIH EMR dev environments (haven't checked production instances but I assume the same as well).

This is due to this pesky issue that we've never figured out how to solve besides not autowiring in bean.

https://issues.openmrs.org/browse/TRUNK-3806

This is a blocker for our development, can we revert this until we figure it out, or at least revert and do a release before adding, because we need @mseaton 's drug CSV enhancement that only in this snapshot. Thanks!

Also fyi @ibacher