I should also mention that this particular feature (exposing data exports from the reportingcompatiblity module as data sets in the reporting module) was always a bit gimmiky and there more to demonstrate the concept of different data persisters than anything else. I would sincerely doubt that anyone actually uses this feature (or knows that it exists). We could likely simply remove it, but I have not done so in order to preserve backwards compatibility.
Perhaps we could simply remove it and release the next version of reporting as (finally) 1.0.0
At first glance it seems easy to test against the different versions, per this: https://wiki.openmrs.org/x/aIC5AQ (at least, I was able to get things to fail to compile against openmrs-core 2.0 because of the removed classes â I donât know know much of a pain it would be to tweak the test datasets).
Would we want to be running builds and tests against the different supported openmrs-core versions in CI? (I assume the answer is yes, though this will of course make the reporting build take 5x longer, for 1.9, 1.10, 1.11, 1.12, and 2.0âŠ)
Yes, @darius I assume it will be easy for us to set up more profiles in the pom. I do think it will be tedious, though not particularly hard, to update the test datasets for each new version of the platform. And I share your question about how and when to run these. There has been a 1.10 profile for a while that is never run, for example, so it isnât doing much good. This is where I was wondering whether Travis CI might be a solution that could help while not running every profile on Bamboo.
I can confirm after spending hours getting the test to run against openmrs-core 1.11 that this is very tedious. (Particularly because prior to 1.11 we never checked referential integrity of test datasets, and the one in reporting has problems.) I have committed this work under https://issues.openmrs.org/browse/REPORT-772
I also created maven profiles for compiling against 1.12 and 2.0, but I havenât updated the tests to work for these yet. However you can see the compile errors against openmrs-core 2.0 if you pull the latest version of the module and do
mvn -P2.0 clean package
I do think we should start running these in CI (i.e. bamboo) for now, and we can think about moving them to another plan, or to travis, later. (Iâll create a plan for the 1.10 and 1.11 builds, which do pass now.)
I have followed on from your work and gotten a 1.12 profile working as well.
For 2.x, I have fixed the issue around reportingcompatibility by moving this code from the main API and into the pre-2.x API only. This seems to have solved that initially reported issue and enables the module to built successfully for the 2.0 profile as long as one skips tests.
For getting tests to pass for the 2.0 profile, I have done some work towards overcoming a bunch of issues, but not all are fully resolved. I have created REPORT-773 to address these remaining issues. I would welcome someone willing to take this on.
Thatâs a pretty good approach, IMO. I mean, you can have a separate plan (totally independent) instead of job, and that would make it not block a release in case itâs red. But I suppose thatâs not what you want in this case.
Having a profile is not a bad approach at all as well. Itâs pretty popular to enable profiles if certain flags are set on the command line, but I think the -P works as good as flags.
Also, I know you are not using it, but as you started with maven profiles do not use activeByDefault. https://dzone.com/articles/maven-profile-best-practices
There are few things to keep in mind:
The current build can possibly upload snapshots to nexus even if the 1.10/1.11/1.12 jobs are red. As Jobs are concurrent, this is a side effect; other builds will use those more recent snapshots even if some of their builds are red. If you want to prevent that, youâd need to make the deployment a serial stage after the others.
The first time a certain job runs on a specific agent, it might take quite a lot of time to download maven dependencies. Iâd expect things to be more or less the same time after a couple of runs on the same machine/user.
We donât have any integration tests that I know of but it seems like good
practice to change to verify instead of package.
Just to check, the secondary jobs that do âverifyâ should not write to the
local maven repo because we havenât ever done install, right? (Really I
mean that there wonât be any bad interaction where anyone else accidentally
uses the artifacts built by the secondary jobs, not the default one.)
That shouldnât be the case anyway, even if you do a âmvn installâ. Bamboo is configured to remove any -SNAPSHOT from the .m2 folder before any build (https://ci.openmrs.org/admin/viewPredatorPlugin.action), so you only have to care about the âmvn deployâ ones, not really âmvn installâ.
As we have two agents running per machine/VM, I forced them to run in two different users, so you never have one build cache affecting the other (assuming we are talking here about cache on the user home).
@mseaton@dkayiwa@darius
I see that there is an new version of reporting omod (0.10.3) released with the changes mentioned above. Does this mean we can use reporting-omod 0.10.3 and ignore reportingcompatibility omod to make it work with Openmrs 2.x?
@mseaton When I tried to use reporting omod (0.10.3) with openmrs 2, I get the following exception:
WARN - AbstractApplicationContext.refresh(487) |2016-10-05 11:55:31,262| Exception encountered during context initialization - cancelling refresh attempt org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'dataExportDataSetDefinitionMappingHandler' defined in URL [jar:file:/opt/openmrs/.openmrs-lib-cache/reporting/reporting.jar!/org/openmrs/module/reporting/web/controller/mapping/DataExportDataSetDefinitionMappingHandler.class]: Initialization of bean failed; nested exception is java.lang.IllegalStateException: Failed to introspect annotations: class org.openmrs.module.reporting.web.controller.mapping.DataExportDataSetDefinitionMappingHandler at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:547) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:476) at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:303) at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230) at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:299) at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:194) at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:762) at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:757) at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:480) at org.openmrs.module.ModuleUtil.refreshApplicationContext(ModuleUtil.java:842) at org.openmrs.module.web.WebModuleUtil.refreshWAC(WebModuleUtil.java:866) at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:653) at org.openmrs.web.Listener.performWebStartOfModules(Listener.java:632) at org.openmrs.web.Listener.startOpenmrs(Listener.java:267) at org.openmrs.web.WebDaemon$1.run(WebDaemon.java:42) Caused by: java.lang.IllegalStateException: Failed to introspect annotations: class org.openmrs.module.reporting.web.controller.mapping.DataExportDataSetDefinitionMappingHandler at org.springframework.core.annotation.AnnotatedElementUtils.process(AnnotatedElementUtils.java:166) at org.springframework.core.annotation.AnnotatedElementUtils.getAnnotationAttributes(AnnotatedElementUtils.java:91) at org.springframework.core.annotation.AnnotatedElementUtils.getAnnotationAttributes(AnnotatedElementUtils.java:85) at org.springframework.transaction.annotation.SpringTransactionAnnotationParser.parseTransactionAnnotation(SpringTransactionAnnotationParser.java:42) at org.springframework.transaction.annotation.AnnotationTransactionAttributeSource.determineTransactionAttribute(AnnotationTransactionAttributeSource.java:154) at org.springframework.transaction.annotation.AnnotationTransactionAttributeSource.findTransactionAttribute(AnnotationTransactionAttributeSource.java:138) at org.springframework.transaction.interceptor.AbstractFallbackTransactionAttributeSource.computeTransactionAttribute(AbstractFallbackTransactionAttributeSource.java:155) at org.springframework.transaction.interceptor.AbstractFallbackTransactionAttributeSource.getTransactionAttribute(AbstractFallbackTransactionAttributeSource.java:100) at org.springframework.transaction.interceptor.TransactionAttributeSourcePointcut.matches(TransactionAttributeSourcePointcut.java:38) at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:225) at org.springframework.aop.support.AopUtils.canApply(AopUtils.java:262) at org.springframework.aop.support.AopUtils.findAdvisorsThatCanApply(AopUtils.java:294) at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findAdvisorsThatCanApply(AbstractAdvisorAutoProxyCreator.java:118) at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.findEligibleAdvisors(AbstractAdvisorAutoProxyCreator.java:88) at org.springframework.aop.framework.autoproxy.AbstractAdvisorAutoProxyCreator.getAdvicesAndAdvisorsForBean(AbstractAdvisorAutoProxyCreator.java:69) at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.wrapIfNecessary(AbstractAutoProxyCreator.java:330) at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessAfterInitialization(AbstractAutoProxyCreator.java:293) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:422) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1571) at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:539) ... 14 more Caused by: java.lang.ArrayStoreException: sun.reflect.annotation.TypeNotPresentExceptionProxy at sun.reflect.annotation.AnnotationParser.parseClassArray(AnnotationParser.java:724) at sun.reflect.annotation.AnnotationParser.parseArray(AnnotationParser.java:531) at sun.reflect.annotation.AnnotationParser.parseMemberValue(AnnotationParser.java:355) at sun.reflect.annotation.AnnotationParser.parseAnnotation2(AnnotationParser.java:286) at sun.reflect.annotation.AnnotationParser.parseAnnotations2(AnnotationParser.java:120) at sun.reflect.annotation.AnnotationParser.parseAnnotations(AnnotationParser.java:72) at java.lang.Class.createAnnotationData(Class.java:3521) at java.lang.Class.annotationData(Class.java:3510) at java.lang.Class.getAnnotations(Class.java:3446) at org.springframework.core.annotation.AnnotatedElementUtils.doProcess(AnnotatedElementUtils.java:192) at org.springframework.core.annotation.AnnotatedElementUtils.process(AnnotatedElementUtils.java:162) ... 33 more
I see that DataExportDataSetDefinition has been moved from api to api 1.9 but this is being used by DataExportDataSetDefinitionMappingHandler in omod submodule. I think it is throwing error since DataExportDataSetDefinition is not available in runtime. How should we go about fixing this?
This is happening because DataExportDataSetDefinitionMappingHandler references DataExportDataSetDefinition which in turn references DataExportReportObject that was moved to the reportingcompatibility module.
