You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2015/06/16 21:45:50 UTC
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/31
ARTEMIS-91 Improvements to the ServiceRegistry interface
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemis master-interface
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/31.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #31
----
commit 42a6394f93aafdfa767a8b122cfc97af921c25f0
Author: Clebert Suconic <cl...@apache.org>
Date: 2015-06-16T19:37:16Z
ARTEMIS-91 Improvements to the ServiceRegistry interface
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/31#discussion_r32566587
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java ---
@@ -32,32 +32,12 @@
{
--- End diff --
It's not really Immutable... the Implementation can decide on how to add / remove...
We just need a contract to load these things... it's up to the implementation how to load / unload these.
On this case this is an application server, and they have their data already loaded somewhere. it could be immutable or not.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by jmesnil <gi...@git.apache.org>.
Github user jmesnil commented on the pull request:
https://github.com/apache/activemq-artemis/pull/31#issuecomment-112713726
this service registry is used by WildFly to pass instance of user-defined classes (such as transformers, interceptors) to Artemis.
WildFly uses a modular class loader so it's its responsibility to instantiate these classes from the correct module. The ServiceRegistry is really just an object repository since Artemis configuration does not allow to pass instances but only class names
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic closed the pull request at:
https://github.com/apache/activemq-artemis/pull/31
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/31#issuecomment-112558308
@johnament / @andytaylor I didn't think it was a good idea to have adds / removes as part of the interface. The implementation can deal with it anyway it wants. For instance I know an application server may have the information somewhere already so it wouldn't need the adds / removes at all..
How the implementation adds / removes would be up to that implementation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/31#issuecomment-112597185
@jmesnil @johnament @andytaylor @jbertram apologies.. I"m an idiot..
I thought Jeff wanted to provide an implementation for the ServiceRegistry on which case it wouldn't make much sense to have the setters...
Reading back from John's comments it seems what we have now is exactly what was needed.
I will just close this PR for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/31#issuecomment-112573215
@jmesnil Can you check this?
I believe it would be up to the implementation to decide to how to set the data, so I removed the setters from the interface...
If you are just dealing with the same instance coming from Artemis, why would you need this abstract? you could then just use the instance itself?
Can you check this pr please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/31#discussion_r32566155
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ServiceRegistry.java ---
@@ -32,32 +32,12 @@
{
--- End diff --
Based on the change in use case (which I don't see in the ticket) should this be `ImmutableServiceRegistry` instead?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request: ARTEMIS-91 Improvements to the Serv...
Posted by andytaylor <gi...@git.apache.org>.
Github user andytaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/31#issuecomment-112554765
I will review this tomorrow if you could leave it for me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---