Re: [Review] GERONIMO-2277 Remove TransactionContextManager

Fri, 04 Aug 2006 17:40:39 -0700

I have a couple of concerns: most of these were probably present in the unmodified code.

1. TransactionManagerImpl.unassociate calls fireThreadAssociated instead of fireThreadUnassociated (dain is fixing this IIUC)

2. ConnectorTransactionContext.managedConnections doesn't have any obvious synchronization.  When I wrote this code I think I had an argument why it wasn't needed, but I'd like to find the object that is in fact guarding it and document it.

3. Am I correct in thinking that you made the connection tracking work without needing instance contexts available?  I think this is a reasonable change, just checking...

4. I wonder if the client still needs a tm since it is inaccessible.

Aside from (1) I understand this change and approve it.  I'll apply and test it out shortly.

Note that (1) does not affect the functionality, as the sole implementations of threadAssociated and threadUnassociated do exactly  the same thing.  (1) should not affect applying this patch.

thanks
david jencks


On Aug 4, 2006, at 4:14 PM, Dain Sundstrom wrote:

I have put together a patch from the notcm branch, and attached it to:




The best way to test this chang is to use svn merge directly instead of using the patch commnd. This is because the svn merge handles adding, removing and relocating files. To merge the changes from the notcm branch execute the following command from within geronimo/trunk


You should not receive any conflicts during the merge. Once this is complete, run the following commands to remove the existing openejb checkout and checkout the new related openejb code (this will be merged directly in openejb):

  rm -rf openejb/
  maven m:co

Then simply run the maven build with the following command:

  maven install

Altermatively, you can use the attached patch insted of the svn merge command.



Please, take a look at this sooner rather than later as the TCM touched lots of code, so this is a large patch.  Currently, it applies without conflict but that could easily change.

Thanks in advance,

-dain

Reply via email to