[GitHub] activemq-artemis pull request #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat R...

classic Classic list List threaded Threaded
40 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    I am working on some compatibility tests I will add into master.. and I will then merge this!


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    Thats fine with me :) , i was just doing and updating you, that i just pushed the example and the docs i promised.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @clebertsuconic - ping - did you get a chance to do what you wanted? you ok to merge this?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    Give me this week ?
   
    I’m adding a new testsuite and I wanted to make sure about this one.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    just updating this that I haven't forgot about this... I'm working on a testsuite for compatibility checks and I will extend it to make sure this will work.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce I just added the testsuite I was after...
   
   
    And as I suspected.. it breaks the serialization wire compatibility.
   
    Isn't there a way we could use writeObject instead of Externalizable? any other way we could fix this?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce perhaps we could add a wrapper for tomcat.. SerialWrapper... so we could add the wrapper on the context.xml instead of the connectionFactory and use read and writeReplace?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    The point here is to be inline with 5.x implementation to make migration for users seemless as possible.
   
    This shouldn’t break any compatibility if it does let me know I’ll update


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    Can we merge this it has been some time now?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce a connection factory serialized with 1.4 won't open with master. Wires are not compatible.
   
    I know for instance that Wildfly uses that...
   
    So, perhaps we could extend ActiveMQConnectionFactory as Ex and use it within Tomcat?
   
   
    if you rebase this... the PR will show the failure as I have included the compatibility-tests on the PR checks... or after rebased go to ./compatibility and run:
   
    mvn -Ptests test
   
   
    if you add the -D classpaths you can run the tests on the IDEs... I added some comments on how to do it.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @clebertsuconic so i run this, but even on master at your last merge (e.g. none of these changes) i get 15 errors in mesh test.



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @clebertsuconic just pushed with fixes, as noted i locally get 15 errors with your new mesh test, but i also get this with master at point of your merge. Serialisation tests pass now.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    just looking at jenkins, seems https://builds.apache.org/job/ActiveMQ-Artemis-PR-Build/4638/ it also failing on same mesh tests, so I'm ignoring that for now.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    Rebase?  I had sent a later fix for the hornetq tests.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @clebertsuconic i had rebased, also i checkout master and it also fails. checking Jenkins it seems to be failing since https://builds.apache.org/job/ActiveMQ-Artemis-PR-Build/4638/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce I am looking what I screwed up :)


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    Can you rebase again and push -f? this will be a good test for me...
   
   
    I had fixed this a couple of times: https://github.com/apache/activemq-artemis/commit/03ed49e6e9f7fb6e465d6d424075ecacba23b3f9
   
   
    But I kept fixing it at the target/classPath instead of the source without realizing it... duh! that's why it wouldn't fail for me.
   
   
    It's actually good it failed... it shows the classLoaders are working correctly as I didn't span new VMs for this.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce In the past, I have used some writeObject / readObject.. that would generate the same wire.. I can't remember the trick now...
   
    When I first wrote this, I thought we wouldn't ever need to make changes to the wire, as it was using properties.. I'm certainly wrong now :)


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat Resource...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1659
 
    @michaelandrepearce already merged this


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1659: ARTEMIS-1516 - Ensure JNDI via Tomcat R...

clebertsuconic-3
In reply to this post by clebertsuconic-3
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1659


---
12