Stream: hapi
Topic: hapi-fhir: pull request 270: Initial commit of the refact...
Zulip HAPI Bot (Dec 15 2015 at 00:56):
bdenton opened pull request 270
James..
this is my initial commit of the RP/DAO refactoring. There is still some more work to do on the XML Spring wiring for OSGi support. Also, I am havint trouble with the Resource Provider tests. Seems to be some sort of issue with Spring/JPA transaction management
Zulip HAPI Bot (Dec 15 2015 at 00:58):
bdenton closed pull request 270
Zulip HAPI Bot (Dec 15 2015 at 00:58):
bdenton reopened pull request 270
James..
this is my initial commit of the RP/DAO refactoring. There is still some more work to do on the XML Spring wiring for OSGi support. Also, I am havint trouble with the Resource Provider tests. Seems to be some sort of issue with Spring/JPA transaction management
Zulip HAPI Bot (Dec 15 2015 at 12:25):
jamesagnew commented on pull request 270
Hi Bill- I see where you're going with this. I'm not totally clear on why it's needed/how it will be used though. Is the idea that you want to have your own implementations of the DAOs that don't use hibernate than then plug those into the resource providers? If so, why do they need to be in their own JAR for that to happen? Couldn't you just create a custom spring configuration that uses the RPs but wires them to a different implementation of the DAO interfaces?
Or am I misunderstanding the motivation?
One other thing to note- this pull request doesn't actually merge cleanly (you can see the
This branch has conflictsmessage below that indicates this). It needs to merge in order for the CI server to build it and the test coverage tools to test it. here are some details on how to fix this...
Zulip HAPI Bot (Dec 15 2015 at 12:33):
jamesagnew commented on pull request 270
One other comment- I'd prefer not to have those hardcoded METADATA.MF files in the repo if we can find a way for them to be generated by maven-bundle-plugin. They have version numbers embedded in them which will get out of date pretty quickly if we don't remember to bump them everytime we release a new version of HAPI.. and I'll guarantee that will get forgotten at some point :)
Zulip HAPI Bot (Dec 15 2015 at 17:50):
bdenton commented on pull request 270
James..
Yes, the reason for the split is to be able to implement RPs and DAOs independently. In the OSGi world, there is good reason to keep them in separate JARs (bundles). Then you use Spring-DM (Gemini Blueprint) to wire across bundles via just interfaces.
I saw that message when I made the pull request and I’ll work on the merge today. It doesn’t look like it will be a big chore.
One other thing, most of the tests in ResourceProviderDstu1Test and ResourceProviderDstu2Test are failing with javax.persistence.TransactionRequiredException. However, all of the FhirResourceDaoXxxx tests succeed… Do you have any ideas why this might occur? Not having worked a bunch with Hibernate and JPA, I am looking for thoughts and ideas.
Cheers, Bill
From: James Agnew [mailto:notifications@github.com]
Sent: Tuesday, December 15, 2015 4:26 AM
To: jamesagnew/hapi-fhir
Cc: Bill Denton
Subject: Re: [hapi-fhir] Initial commit of the refactored Resource Provider and DAO layers (#270)Hi Bill- I see where you're going with this. I'm not totally clear on why it's needed/how it will be used though. Is the idea that you want to have your own implementations of the DAOs that don't use hibernate than then plug those into the resource providers? If so, why do they need to be in their own JAR for that to happen? Couldn't you just create a custom spring configuration that uses the RPs but wires them to a different implementation of the DAO interfaces?
Or am I misunderstanding the motivation?
One other thing to note- this pull request doesn't actually merge cleanly (you can see the This branch has conflicts message below that indicates this). It needs to merge in order for the CI server to build it and the test coverage tools to test it. here<https://help.github.com/articles/syncing-a-fork/> are some details on how to fix this...
—
Reply to this email directly or view it on GitHub<https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164749248>.
Zulip HAPI Bot (Dec 15 2015 at 17:59):
bdenton commented on pull request 270
The METADATA.MF file is there so the project can be managed as a Plugin Project in Eclipse. This makes resolving package imports so much easier. I only put version numbers on the imports when it is actually needed. In OSGi, specifying version=”x.y.z” is just saying what the minimum version needs to be.. not the specific version. For instance, in hapi-fhir-base, the imports of org.apache.http.* specify version=”4.0” because the code references methods that were introduced in v4.0.
One thing I really don’t care for in the Maven bundle plugin is that it wants to put a version on every import and this is usually the latest version of things. The problem with this is it makes it really hard to install the bundle in an existing OSGi container that might not have that exact version in place. For things that are pretty generic like commons-io or commons-lang3, the version is not really useful. For things like javax.servlet, it can actually make it impossible to deploy if the version does not match the version of the underlying platform.
Like I said, I only use the versions when code references something that was not part of the base version of a package. Also, I pretty much never use “bracketing” version specifications (version=”(1.0,2.0]” which means version=1.x.x) since nearly everything has pretty good downward compatibility (v2.x will usually be compatible with code written for v1.x).
Cheers, Bill
From: James Agnew [mailto:notifications@github.com]
Sent: Tuesday, December 15, 2015 4:34 AM
To: jamesagnew/hapi-fhir
Cc: Bill Denton
Subject: Re: [hapi-fhir] Initial commit of the refactored Resource Provider and DAO layers (#270)One other comment- I'd prefer not to have those hardcoded METADATA.MF files in the repo if we can find a way for them to be generated by maven-bundle-plugin. They have version numbers embedded in them which will get out of date pretty quickly if we don't remember to bump them everytime we release a new version of HAPI.. and I'll guarantee that will get forgotten at some point :)
—
Reply to this email directly or view it on GitHub<https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164750721>.
Zulip HAPI Bot (Dec 15 2015 at 21:21):
bdenton synchronized pull request 270
Zulip HAPI Bot (Dec 15 2015 at 21:42):
bdenton synchronized pull request 270
Zulip HAPI Bot (Dec 15 2015 at 22:28):
bdenton commented on pull request 270
James..
I followed the instructions above and the refactoring branch is now merged into the master in my fork. I did an upstream merge prior to merging in the branch. Not a GIT expert by any means so I am hoping I did the right thing.
BTW, some other commits I made to the hapi-osgi-core project slipped in as well
cheers, Bill
Zulip HAPI Bot (Dec 15 2015 at 22:28):
bdenton commented on pull request 270
I think I need to close this pull request and open a new one on the master branch??
Zulip HAPI Bot (Dec 15 2015 at 22:28):
bdenton closed pull request 270
Zulip HAPI Bot (Dec 16 2015 at 02:51):
jamesagnew commented on pull request 270
Hmm no, unfortunately I have no idea why that would be happening.
I'll try to check it out and run it myself at some point soon and see if I
see anything obvious.I think you need to add the new dao-base project to the list of modules in
the ALLMODULES profile in the root pom.xml. The CI server is failing
<https://travis-ci.org/jamesagnew/hapi-fhir/builds/97096585> but I think
that's just because it's not finding that module.Cheers,
JamesOn Tue, Dec 15, 2015 at 12:56 PM, Bill Denton <notifications@github.com>
wrote:James..
Yes, the reason for the split is to be able to implement RPs and DAOs
independently. In the OSGi world, there is good reason to keep them in
separate JARs (bundles). Then you use Spring-DM (Gemini Blueprint) to wire
across bundles via just interfaces.I saw that message when I made the pull request and I’ll work on the merge
today. It doesn’t look like it will be a big chore.One other thing, most of the tests in ResourceProviderDstu1Test and
ResourceProviderDstu2Test are failing with
javax.persistence.TransactionRequiredException. However, all of the
FhirResourceDaoXxxx tests succeed… Do you have any ideas why this might
occur? Not having worked a bunch with Hibernate and JPA, I am looking for
thoughts and ideas.Cheers, Bill
From: James Agnew [mailto:notifications@github.com]
Sent: Tuesday, December 15, 2015 4:26 AM
To: jamesagnew/hapi-fhir
Cc: Bill Denton
Subject: Re: [hapi-fhir] Initial commit of the refactored Resource
Provider and DAO layers (#270)Hi Bill- I see where you're going with this. I'm not totally clear on why
it's needed/how it will be used though. Is the idea that you want to have
your own implementations of the DAOs that don't use hibernate than then
plug those into the resource providers? If so, why do they need to be in
their own JAR for that to happen? Couldn't you just create a custom spring
configuration that uses the RPs but wires them to a different
implementation of the DAO interfaces?Or am I misunderstanding the motivation?
One other thing to note- this pull request doesn't actually merge cleanly
(you can see the This branch has conflicts message below that indicates
this). It needs to merge in order for the CI server to build it and the
test coverage tools to test it. here<
https://help.github.com/articles/syncing-a-fork/> are some details on how
to fix this...—
Reply to this email directly or view it on GitHub<
https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164749248>.—
Reply to this email directly or view it on GitHub
<https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164839810>.
Zulip HAPI Bot (Dec 16 2015 at 03:09):
bdenton commented on pull request 270
Yes, I found that out when I finally got the merge to master finished… also need to add hapi-fhir-providers
Not sure it is needed but I also added them to SITE <profile>
From: James Agnew [mailto:notifications@github.com]
Sent: Tuesday, December 15, 2015 6:52 PM
To: jamesagnew/hapi-fhir
Cc: Bill Denton
Subject: Re: [hapi-fhir] Initial commit of the refactored Resource Provider and DAO layers (#270)Hmm no, unfortunately I have no idea why that would be happening.
I'll try to check it out and run it myself at some point soon and see if I
see anything obvious.I think you need to add the new dao-base project to the list of modules in
the ALLMODULES profile in the root pom.xml. The CI server is failing
<https://travis-ci.org/jamesagnew/hapi-fhir/builds/97096585> but I think
that's just because it's not finding that module.Cheers,
JamesOn Tue, Dec 15, 2015 at 12:56 PM, Bill Denton <notifications@github.com<mailto:notifications@github.com>>
wrote:James..
Yes, the reason for the split is to be able to implement RPs and DAOs
independently. In the OSGi world, there is good reason to keep them in
separate JARs (bundles). Then you use Spring-DM (Gemini Blueprint) to wire
across bundles via just interfaces.I saw that message when I made the pull request and I’ll work on the merge
today. It doesn’t look like it will be a big chore.One other thing, most of the tests in ResourceProviderDstu1Test and
ResourceProviderDstu2Test are failing with
javax.persistence.TransactionRequiredException. However, all of the
FhirResourceDaoXxxx tests succeed… Do you have any ideas why this might
occur? Not having worked a bunch with Hibernate and JPA, I am looking for
thoughts and ideas.Cheers, Bill
From: James Agnew [mailto:notifications@github.com]
Sent: Tuesday, December 15, 2015 4:26 AM
To: jamesagnew/hapi-fhir
Cc: Bill Denton
Subject: Re: [hapi-fhir] Initial commit of the refactored Resource
Provider and DAO layers (#270)Hi Bill- I see where you're going with this. I'm not totally clear on why
it's needed/how it will be used though. Is the idea that you want to have
your own implementations of the DAOs that don't use hibernate than then
plug those into the resource providers? If so, why do they need to be in
their own JAR for that to happen? Couldn't you just create a custom spring
configuration that uses the RPs but wires them to a different
implementation of the DAO interfaces?Or am I misunderstanding the motivation?
One other thing to note- this pull request doesn't actually merge cleanly
(you can see the This branch has conflicts message below that indicates
this). It needs to merge in order for the CI server to build it and the
test coverage tools to test it. here<
https://help.github.com/articles/syncing-a-fork/> are some details on how
to fix this...—
Reply to this email directly or view it on GitHub<
https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164749248>.—
Reply to this email directly or view it on GitHub
<https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164839810>.—
Reply to this email directly or view it on GitHub<https://github.com/jamesagnew/hapi-fhir/pull/270#issuecomment-164969546>.
Last updated: Apr 12 2022 at 19:14 UTC
