It’s very good that we can have a concise set of metadata (brought in by the reference application configuration) and some accompanying demodata along-side to demonstrate the capabilities of OpenMRS Reference application, and for devs to work against on a demo instance like dev3.openmrs.org.
Unfortunately while improvements are made on the metadata, we face some limitations, 1.) difficulty in reviewing managing and maintaining 100% error-free ocl packages (because they are zipped in PRs), 2.) insufficient validation on the metadata/configuration introduced/changed to make sure nothing is breaking demodata generation or that at-least a certain set of rules are met identical to real-world use-cases and don’t violate the OpenMRS api restrictions. I’ve faced some failure cases running the current Reference Application distro.
IMO if there was some kind of way to have configuration pull-requests carefully tested or at-least validated before merging, we’d reduce failure-surprises and the time it’d take figuring out what went wrong (like can be the case for ocl package failures), if this can be checked way before a PR is merged.
So, I said this on Slack, but to re-iterate here, I think one issue is that the demo data module, in it’s current incarnation, has undocumented dependencies on the way metadata is organised. Requiring that metadata authors check for undocumented dependencies, I think, gets things exactly backwards. Instead, in my view, the demo data module is currently relying on metadata conforming to certain rules without validating that the metadata conforms to those rules.
The detailed cause of the current error is that there is a LabSet concept set without any defined set members. This results in the demo data module trying to generate an obs group without any value obs in it. The resulting “obs group” obs doesn’t have a value, nor is it an “obs group” from the validators perspective, so creating the group fails.
There are, I think, two obvious fixes:
In this code, we should verify that the “LabSet” concept set actually has set members before trying to use it.
We don’t just grab any random “LabSet” concept, but rather deal with a list of validated “LabSet” concepts.
Either of those two would solve the immediate problem.
On a longer-term basis, though, I agree that we need to fix things, but ultimately, it’s on the demo data module to maintain compatibility with the current metadata and not metadata authors responsibility to maintain compatibility with the demo data. Its perfectly reasonable, for example, for the RefApp to have concepts and concept sets that the demo data module can’t work with; more to the point, though, the metadata we merge into the RefApp should not have to conform to the expectations of a module that exists only for demo purposes and would not be active in production…