Currently, emrapi depends on reporting, providermanagement, metadatasharing, event, webservices and appframework. Bahmni just about needs providermanagement functionalities. Is it alright to make all of these aware of modules instead of require modules?
@shruthidipali I didn’t quite understand from your email which ones Bahmni does need vs does not.
Here’s my recollection of the reasoning behind these, and current thoughts:
webservices.rest
this is a core part of the platform, and this dependency should stay
providermanagement
this is a too-heavy way of providing “provider type” in the reference application.
ideally we would add provider type to openmrs-core, and remove this module from the reference application (and remove this dependency from emrapi)
metadatasharing
emrapi provides some utility methods to load MDS packages
but really those methods ought to live somewhere else => this dependency is misguided, and I’d welcome someone doing the work to move the code to another module
event
@wyclif added this dependency to support recently-viewed patients. See RA-200.
I could live with having that feature moved to another module, but I’m not sure which one. But I’d prefer to leave this where it is.
appframework
I never wanted this dependency – uiframework, appframework, and emrapi were all supposed to be the independent base pieces
@mogoodrich added it to support feature toggles. see EA-13, where he says “If we change our mind, we can always pull this dependency out later.”
reporting
the original intent was that each of the higher-level constructs that are built on top of obs (e.g. Diagnoses) should reporting/query functionality alongside them in the same module.
I might be mistaken, but I’m pretty sure the metadatasharing utility
methods should be removed from emrapi, as they are re-introduced (and
better belong) in the metadatadeploy module. But someone would need to
confirm.
Thanks @darius it looks like all feature toggle references have been removed from emr-api, but I neglected to remove the dependency on appframework itself. I just pulled it from the pom and the module still compiles and passes, so I’ll commit the change.
@shruthidipali are you asking if we can just make the modules you don’t use “aware of” in the config.xml so emr-api can be started without them even those the dependency still exists, I think that’s something we want to avoid.
@darius@mogoodrich I was wondering if we can make all dependant modules to be “aware of” modules in emrapi module’s config.xml. we would save some startup time that way.
In general I don’t like that idea. They are real dependencies for the
module, and I think we are a lot better off having these be explicit, and
fail fast, even if it increases startup time a bit.
Looking at the specifics:
webservices.rest => everyone should be running this module, so it gains
nothing to make it aware-of
metadatasharing => the correct thing is to refactor and remove this
dependency
appframework => @mogoodrich already removed this dependency
providermanagement => I suspect this will break things to remove (and the
correct fix is to add one field to openmrs-core)
The last two are “event” and “reporting”. I guess I could live with these
being optional dependencies. E.g. if you don’t have the reporting module
loaded, then presumably nothing will try to use the reporting/querying
classes. And I suppose it could be okay to not fire events if the event
module isn’t loaded.
Even so, I would vote against making these changes for the sake of startup
time (it really feels like a hack), and instead focus on the real
improvements we have identified around removing dependencies.
Any thoughts on further dividing emrapi into smaller omods? that way, the implementation is free to choose a smaller set of dependencies. We could also have a blanket omod which includes everything as well. The choice would then be that of an implementer to either include everything or just a subset of the dependencies.
If Bahmni wants to do a fork of emrapi that has fewer dependencies, that’s
fine (and it’s fine if this is not a code fork, but happens as part of the
build process of that module). But I don’t approve of releasing multiple
smaller variations of emrapi, because the confusion this introduces
generally will outweigh the benefits of slightly-improved startup times.
But go back to my previous email: most of these dependencies are okay to
remove, and I would really prefer that we remove them in the right way,
rather than attempt hacks that will probably take more time to implement
and test.
As I said:
remove metadatasharing by moving some utility methods to the
metadatadeploy module instead
remove providermanagment by adding a “provider type” in openmrs-core
event and reporting could be aware-of dependencies if you really want