You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/11/15 14:25:54 UTC
[GitHub] [activemq-artemis] MM53 opened a new pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
MM53 opened a new pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853
* Add BindingDTO to allow configuring multiple addresses to listen on
* Start a new ServerConnector for each binding and deploy the corresponding web-applications
* Update documentation and tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994041405
@MM53 an easy way to reproduce it if you don't want to try on the IDE is by running the following command from the ./smoke-tests directory:
```
cd activemq-artemis/tests/smoke-tests
../../one-test.sh ConsoleMutualSSLTest
```
if you don't like using the one-test.sh (which is a script I added because I am not good to remember the actual command when I need it fast:
```
cd activemq-artemis/tests/smoke-tests
mvn -Ptests -DfailIfNoTests=false -Ptests-retry -Ptests-CI -Pextra-tests -DskipStyleCheck=true -DskipPerformanceTests=false -Dtest=ConsoleMutualSSLTest test
```
(you will probably understand why I committed one-test.sh)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993262769
@jbertram Thanks for your help. I rebased my branch onto the current main state. So all conflicts should be resolved now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic edited a comment on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic edited a comment on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994042919
I am actually postponing the release for a day.. It's too late in the day for me to upload it.. so I will do it tomorrow.
I don't mean to pressure you.. but if you fix it today I will merge it and put it on the release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-971645721
I updated my changes to work properly with the old style configuration and added some tests to verify both formats. I also resolved the checkstyle violations. Hope everything is fine now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994029727
I will have a look at it and create a new PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993604616
@clebertsuconic, do you mind if I merge this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994010675
@clebertsuconic where did you get an error or how could I reproduce it? When I use maven to build a release locally there are no failures in the smoke tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-969165228
@jbertram, thanks for your response. My plan was to keep backwards compatibility but I forgot to check for null instead I only checked if the list contains some bindings. I will fix this issue and test the code with some previous configurations to see if there are more problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-969131667
In general I like this idea since the underlying Jetty server supports this kind of behavior natively. I was concerned at first that supporting this would require creating two instances of Jetty, but that is not the case. This is good work, @MM53!
The problem is that there is no backwards compatibility with the previous configuration. Using the previous configuration results in an NPE, i.e.:
```
java.lang.NullPointerException
at org.apache.activemq.artemis.dto.WebServerDTO.getBindings(WebServerDTO.java:96)
at org.apache.activemq.artemis.component.WebServerComponent.configure(WebServerComponent.java:87)
at org.apache.activemq.artemis.cli.commands.Run.execute(Run.java:125)
at org.apache.activemq.artemis.cli.Artemis.internalExecute(Artemis.java:157)
at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:105)
at org.apache.activemq.artemis.cli.Artemis.execute(Artemis.java:132)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.apache.activemq.artemis.boot.Artemis.execute(Artemis.java:134)
at org.apache.activemq.artemis.boot.Artemis.main(Artemis.java:50)
```
This is a *breaking* change and we cannot introduce it in a minor release. It would need to wait until the next major release.
Alternatively you could provide backwards compatibility and then I think this could be merged and used in the next minor release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994111953
@clebertsuconic there was a bug while generating the configuration for a new server which mixed up the old and new configuration style. Therefore the keyStore and trustStore parameters were ignored. I fixed this and created a new PR: #3882 containing all changes.
Not sure why the maven build didn't fail because of this but now everything should work as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-969180401
@MM53, that's good news. It would probably be worth leaving some tests for the old style configuration and creating new tests for the new config. That way the test-suite will exercise both options and problems like this would be detected automatically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] MM53 commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
MM53 commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994070311
@clebertsuconic thanks, I will try to fix the problem before you build the actual release
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-969132706
FWIW, there are a handful of checkstyle violations. I recommend you run `mvn compile -Pdev` and inspect the results locally or check out the results of the automated PR builds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994042919
I am actually postponing the release for a day.. It's too late in the day for me to upload it.. so I will do it tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993604374
I just verified that these changes don't break existing/upgrading users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994013575
The failing test-case was already noted - `org.apache.activemq.artemis.tests.smoke.console.ConsoleMutualSSLTest`. It fails for me when I run it in my IDE.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-994045801
also, notice smoke-tests use real servers.. so you may have to build the whole project when you make updates to the codebase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993949175
@jbertram / @MM53 I am reverting this... there's a failure from org.apache.activemq.artemis.tests.smoke.console.ConsoleMutualSSLTest that looks real to me
I would rather defer the change now than break the release..
Can one of you send a new PR with the fix later please?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993567992
@jbertram / @MM53 what's the status on this?
I'm going to release today, but this is a big change to include right before the release... I will leave it out for now..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic merged pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
clebertsuconic merged pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3853: ARTEMIS-3574 multiple bindings for embedded webserver
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3853:
URL: https://github.com/apache/activemq-artemis/pull/3853#issuecomment-993132047
@MM53, there's a conflict now. It's a simple change from c502e94ade9e050849a2d7fd2431c28ea77f4f0a. Once you resolve the conflict I'll merge the PR. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org