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 2022/04/04 13:07:59 UTC
[GitHub] [activemq-artemis] ehsavoie opened a new pull request, #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
ehsavoie opened a new pull request, #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009
* passing the full list of parameters when creating the URI.
* moving the WARN message to DEBUG when unknown parameters are
received.
Jira: https://issues.apache.org/jira/browse/ARTEMIS-3756
--
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 a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r845154841
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java:
##########
@@ -425,10 +425,10 @@
format = Message.Format.MESSAGE_FORMAT)
void broadcastTimeout(int retry, int maxretry);
- @LogMessage(level = Logger.Level.WARN)
- @Message(id = 212078, value = "Connection factory parameter ignored {0}",
- format = Message.Format.MESSAGE_FORMAT)
- void connectionFactoryParameterIgnored(String parameterName);
Review Comment:
Can you just remove this, and call logger.debug directly where this is being called.
We always use logger.debug at the logger on the class.
--
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] gemmellr commented on pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
gemmellr commented on PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#issuecomment-1106478490
Ah, missed that the code was changed, yep seems likely it could be related hehe.
@clebertsuconic is the fix to change the logging again, or alter the test?
--
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] ehsavoie commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r856150347
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
+ LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").setLevel(Level.ALL);
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "tcp://localhost:61616?foo=too");
AssertionLoggerHandler.startCapture();
try {
- new InitialContext(props);
- assertTrue("Expected to find AMQ212078", AssertionLoggerHandler.findText("AMQ212078"));
+ Context ctx = new InitialContext(props);
+ ctx.close();
+ assertTrue("Connection factory parameter foo is not standard", AssertionLoggerHandler.findText("Connection factory parameter foo is not standard"));
Review Comment:
The trace was moved to debug, could it be some logging misconfiguration ?
--
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 a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r845155383
##########
artemis-jms-client/src/main/java/org/apache/activemq/artemis/uri/TCPSchema.java:
##########
@@ -83,7 +83,7 @@ private void checkIgnoredQueryFields(ActiveMQConnectionFactory factory, Map<Stri
if (!key.equals("ha") && !key.equals("type") &&
!TransportConstants.ALLOWABLE_CONNECTOR_KEYS.contains(key) &&
!factoryProperties.containsKey(key)) {
- ActiveMQClientLogger.LOGGER.connectionFactoryParameterIgnored(key);
+ ActiveMQClientLogger.LOGGER.connectionFactoryExtraParameter(key);
Review Comment:
just define a logger on TCPSchema, can call logger.debug here directly...
--
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] ehsavoie commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r845163811
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java:
##########
@@ -425,10 +425,10 @@
format = Message.Format.MESSAGE_FORMAT)
void broadcastTimeout(int retry, int maxretry);
- @LogMessage(level = Logger.Level.WARN)
- @Message(id = 212078, value = "Connection factory parameter ignored {0}",
- format = Message.Format.MESSAGE_FORMAT)
- void connectionFactoryParameterIgnored(String parameterName);
Review Comment:
Is it ok to remove the id 212078 or should I keep it in the debug trace ? There is a test looking for it.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
Review Comment:
ok
--
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 a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r845156155
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
Review Comment:
lets just remove this test?
--
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] gemmellr commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
gemmellr commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r856125596
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
+ LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").setLevel(Level.ALL);
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "tcp://localhost:61616?foo=too");
AssertionLoggerHandler.startCapture();
try {
- new InitialContext(props);
- assertTrue("Expected to find AMQ212078", AssertionLoggerHandler.findText("AMQ212078"));
+ Context ctx = new InitialContext(props);
+ ctx.close();
+ assertTrue("Connection factory parameter foo is not standard", AssertionLoggerHandler.findText("Connection factory parameter foo is not standard"));
Review Comment:
This assertion is failing when the test is run (and then afterwards the test sits around waiting for threads to go away).
EDIT: I now see a prior comment about actually removing the test. Not sure if there was other context around that but clearly its still here.
--
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] asfgit closed pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
URL: https://github.com/apache/activemq-artemis/pull/4009
--
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] ehsavoie commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r845164315
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/ActiveMQClientLogger.java:
##########
@@ -425,10 +425,10 @@
format = Message.Format.MESSAGE_FORMAT)
void broadcastTimeout(int retry, int maxretry);
- @LogMessage(level = Logger.Level.WARN)
- @Message(id = 212078, value = "Connection factory parameter ignored {0}",
- format = Message.Format.MESSAGE_FORMAT)
- void connectionFactoryParameterIgnored(String parameterName);
Review Comment:
ok just saw your last comment.
--
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] ehsavoie commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r856152230
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
+ LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").setLevel(Level.ALL);
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "tcp://localhost:61616?foo=too");
AssertionLoggerHandler.startCapture();
try {
- new InitialContext(props);
- assertTrue("Expected to find AMQ212078", AssertionLoggerHandler.findText("AMQ212078"));
+ Context ctx = new InitialContext(props);
+ ctx.close();
+ assertTrue("Connection factory parameter foo is not standard", AssertionLoggerHandler.findText("Connection factory parameter foo is not standard"));
Review Comment:
The code has been changed from ActiveMQClientLogger.LOGGER.debug(String.format("Connection factory parameter %s is not standard", key)); to logger.debugf("Connection factory parameter %s is not standard", key); private static final Logger logger = Logger.getLogger(TCPSchema.class); which is a different logger category !!! this explains that
--
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] ehsavoie commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r856150871
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
+ LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").setLevel(Level.ALL);
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "tcp://localhost:61616?foo=too");
AssertionLoggerHandler.startCapture();
try {
- new InitialContext(props);
- assertTrue("Expected to find AMQ212078", AssertionLoggerHandler.findText("AMQ212078"));
+ Context ctx = new InitialContext(props);
+ ctx.close();
+ assertTrue("Connection factory parameter foo is not standard", AssertionLoggerHandler.findText("Connection factory parameter foo is not standard"));
Review Comment:
https://github.com/apache/activemq-artemis/pull/4009/files#diff-c86b5f721ed580116525565937bfbc27f4dfcbc64c21e43a55a94780b4d90e73R20
--
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] gemmellr commented on a diff in pull request #4009: [ARTEMIS-3756]: Enabling extra parameters to be passed by the URI.
Posted by GitBox <gi...@apache.org>.
gemmellr commented on code in PR #4009:
URL: https://github.com/apache/activemq-artemis/pull/4009#discussion_r856193788
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java:
##########
@@ -111,22 +115,26 @@ public void testEmptyConnectionFactoryString() throws NamingException, JMSExcept
//IIB v10 assumes this property is mandatory and sets it to an empty string when not specified
props.put("java.naming.provider.url", "");
- new InitialContext(props);//Must not throw an exception
-
+ Context ctx = new InitialContext(props);//Must not throw an exception
+ ctx.close();
}
@Test
public void testConnectionFactoryStringWithInvalidParameter() throws Exception {
+ Level initialLevel = LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").getLevel();
+ LogManager.getLogManager().getLogger("org.apache.activemq.artemis.core.client").setLevel(Level.ALL);
Hashtable<String, String> props = new Hashtable<>();
props.put(Context.INITIAL_CONTEXT_FACTORY, "org.apache.activemq.artemis.jndi.ActiveMQInitialContextFactory");
props.put("connectionFactory.ConnectionFactory", "tcp://localhost:61616?foo=too");
AssertionLoggerHandler.startCapture();
try {
- new InitialContext(props);
- assertTrue("Expected to find AMQ212078", AssertionLoggerHandler.findText("AMQ212078"));
+ Context ctx = new InitialContext(props);
+ ctx.close();
+ assertTrue("Connection factory parameter foo is not standard", AssertionLoggerHandler.findText("Connection factory parameter foo is not standard"));
Review Comment:
Ah, missed that the code was changed, yep seems likely it could be related hehe.
@clebertsuconic is the fix to change the logging again, or alter the test?
--
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