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/05 22:24:22 UTC
[GitHub] [activemq-artemis] erwindon opened a new pull request #3841: ARTEMIS-3556 show message protocol type on message view page
erwindon opened a new pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841
see https://issues.apache.org/jira/browse/ARTEMIS-3556
--
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 #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841
--
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] erwindon commented on a change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
erwindon commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r745124317
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
maybe I should squash to prevent confusion?
--
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] erwindon commented on a change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
erwindon commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r744113773
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
please ignore the remark about getClass().getSimpleName(). there are only very few internal classes that are not a sub of CoreMessage. so that problem is negligable.
--
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] erwindon commented on a change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
erwindon commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r745123812
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
I already made that change exactly as you and Michael suggest. that code is in the next commit.
--
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] erwindon commented on a change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
erwindon commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r744113773
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
please ignore the remark about getClass().getSimpleName(). there is only one internal class that is not a sub of CoreMessage. so that problem is negligable.
--
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] michaelandrepearce commented on a change in pull request #3841: ARTEMIS-3556 show message protocol type on message view page
Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r744110439
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
This is a bit dangerous as it exposes class name to api, can we make this something more explicit e.g. add interface to message api such as (getProtocol) and then for the protocol implementation classes to implement that returning a constant value.
--
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 change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r745096534
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
we could add a method on Message to return a more user friendly word?
on Message:
public String getProtocol();
then on core you return "CORE" and AMQP you return "AMQP"
?
--
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] erwindon commented on a change in pull request #3841: ARTEMIS-3556 show message protocol on message view page
Posted by GitBox <gi...@apache.org>.
erwindon commented on a change in pull request #3841:
URL: https://github.com/apache/activemq-artemis/pull/3841#discussion_r744111895
##########
File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/message/openmbean/MessageOpenTypeFactory.java
##########
@@ -123,6 +124,7 @@ protected void init() throws OpenDataException {
public Map<String, Object> getFields(M m, int valueSizeLimit, int deliveryCount) throws OpenDataException {
Map<String, Object> rc = new HashMap<>();
rc.put(CompositeDataConstants.MESSAGE_ID, "" + m.getMessageID());
+ rc.put(CompositeDataConstants.PROTOCOL, m.getClass().getSimpleName());
Review comment:
That was my original thought, until I saw how self-descriptive the class-names were.
I'll follow the original thought and your suggestion.
Still, I'll make the default implementation getClass().getSimpleName() because there are internal classes and client classes for which the name is never shown.
--
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