You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by graben <gi...@git.apache.org> on 2016/10/11 19:28:48 UTC

[GitHub] activemq-artemis pull request #839: ARTEMIS-793 Improvement to OSGi integrat...

GitHub user graben opened a pull request:

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

    ARTEMIS-793 Improvement to OSGi integration

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/graben/activemq-artemis master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/839.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 #839
    
----
commit 021fa843cd2ccabb2be509df18ebb57c8ab260ed
Author: Benjamin Graf <be...@gmx.net>
Date:   2016-10-11T19:27:52Z

    ARTEMIS-793 Improvement to OSGi integration

----


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    Well, I think it must be the same callback to avoid the protocol tracker to start the broker if the datasource is not bounded yet. It's just another "mandatory" dependency. I might rename the class to more general name, but separating would end up in a mass.


---
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 #839: ARTEMIS-793 Improvement to OSGi integrat...

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/839#discussion_r83230546
  
    --- Diff: artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/OsgiBroker.java ---
    @@ -1,223 +1,252 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one or more
    - * contributor license agreements. See the NOTICE file distributed with
    - * this work for additional information regarding copyright ownership.
    - * The ASF licenses this file to You under the Apache License, Version 2.0
    - * (the "License"); you may not use this file except in compliance with
    - * the License. You may obtain a copy of the License at
    - *
    - *     http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -package org.apache.activemq.artemis.osgi;
    -
    -import java.io.File;
    -import java.lang.management.ManagementFactory;
    -import java.util.ArrayList;
    -import java.util.Dictionary;
    -import java.util.Enumeration;
    -import java.util.HashMap;
    -import java.util.HashSet;
    -import java.util.Hashtable;
    -import java.util.List;
    -import java.util.Map;
    -import java.util.Set;
    -
    -import org.apache.activemq.artemis.api.core.Interceptor;
    -import org.apache.activemq.artemis.api.core.TransportConfiguration;
    -import org.apache.activemq.artemis.core.config.FileDeploymentManager;
    -import org.apache.activemq.artemis.core.config.impl.FileConfiguration;
    -import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
    -import org.apache.activemq.artemis.core.server.ActiveMQComponent;
    -import org.apache.activemq.artemis.core.server.ActiveMQServer;
    -import org.apache.activemq.artemis.jms.server.config.impl.FileJMSConfiguration;
    -import org.apache.activemq.artemis.spi.core.protocol.ProtocolManagerFactory;
    -import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager;
    -import org.osgi.framework.BundleContext;
    -import org.osgi.framework.ServiceRegistration;
    -import org.osgi.service.component.ComponentContext;
    -import org.osgi.service.component.annotations.Activate;
    -import org.osgi.service.component.annotations.Component;
    -import org.osgi.service.component.annotations.ConfigurationPolicy;
    -import org.osgi.service.component.annotations.Deactivate;
    -import org.osgi.util.tracker.ServiceTracker;
    -
    -@SuppressWarnings({"unchecked", "rawtypes"})
    -@Component(configurationPid = "org.apache.activemq.artemis", configurationPolicy = ConfigurationPolicy.REQUIRE)
    -public class OsgiBroker {
    -
    -   private String name;
    -   private String configurationUrl;
    -   private String rolePrincipalClass;
    -   private Map<String, ActiveMQComponent> components;
    -   private Map<String, ServiceRegistration<?>> registrations;
    -   private ServiceTracker tracker;
    -
    -   @Activate
    -   public void activate(ComponentContext cctx) throws Exception {
    -      final BundleContext context = cctx.getBundleContext();
    -      final Dictionary<String, Object> properties = cctx.getProperties();
    -      configurationUrl = getMandatory(properties, "config");
    -      name = getMandatory(properties, "name");
    -      rolePrincipalClass = (String) properties.get("rolePrincipalClass");
    -      String domain = getMandatory(properties, "domain");
    -      ActiveMQJAASSecurityManager security = new ActiveMQJAASSecurityManager(domain);
    -      if (rolePrincipalClass != null) {
    -         security.setRolePrincipalClass(rolePrincipalClass);
    -      }
    -      String brokerInstance = null;
    -      String karafDataDir = System.getProperty("karaf.data");
    -      if (karafDataDir != null) {
    -         brokerInstance = karafDataDir + "/artemis/" + name;
    -      }
    -
    -      // todo if we start to pullout more configs from the main config then we
    -      // should pull out the configuration objects from factories if available
    -      FileConfiguration configuration = new FileConfiguration();
    -      if (brokerInstance != null) {
    -         configuration.setBrokerInstance(new File(brokerInstance));
    -      }
    -      FileJMSConfiguration jmsConfiguration = new FileJMSConfiguration();
    -
    -      FileDeploymentManager fileDeploymentManager = new FileDeploymentManager(configurationUrl);
    -      fileDeploymentManager.addDeployable(configuration).addDeployable(jmsConfiguration).readConfiguration();
    -
    -      components = fileDeploymentManager.buildService(security, ManagementFactory.getPlatformMBeanServer());
    -
    -      final ActiveMQServer server = (ActiveMQServer) components.get("core");
    -
    -      String[] requiredProtocols = getRequiredProtocols(server.getConfiguration().getAcceptorConfigurations());
    -      ProtocolTrackerCallBack callback = new ProtocolTrackerCallBack() {
    -
    -         @Override
    -         public void addFactory(ProtocolManagerFactory<Interceptor> pmf) {
    -            server.addProtocolManagerFactory(pmf);
    -         }
    -
    -         @Override
    -         public void removeFactory(ProtocolManagerFactory<Interceptor> pmf) {
    -            server.removeProtocolManagerFactory(pmf);
    -         }
    -
    -         @Override
    -         public void stop() throws Exception {
    -            ActiveMQComponent[] mqComponents = new ActiveMQComponent[components.size()];
    -            components.values().toArray(mqComponents);
    -            for (int i = mqComponents.length - 1; i >= 0; i--) {
    -               mqComponents[i].stop();
    -            }
    -            unregister();
    -         }
    -
    -         @Override
    -         public void start() throws Exception {
    -            List<ActiveMQComponent> componentsByStartOrder = getComponentsByStartOrder(components);
    -            for (ActiveMQComponent component : componentsByStartOrder) {
    -               component.start();
    -            }
    -            register(context, properties);
    -         }
    -
    -         @Override
    -         public boolean isStarted() {
    -            return server.isStarted();
    -         }
    -      };
    -      ProtocolTracker trackerCust = new ProtocolTracker(name, context, requiredProtocols, callback);
    -      tracker = new ServiceTracker(context, ProtocolManagerFactory.class, trackerCust);
    -      tracker.open();
    -   }
    -
    -   private String getMandatory(Dictionary<String, ?> properties, String key) {
    -      String value = (String) properties.get(key);
    -      if (value == null) {
    -         throw new IllegalStateException("Property " + key + " must be set");
    -      }
    -      return value;
    -   }
    -
    -   private String[] getRequiredProtocols(Set<TransportConfiguration> acceptors) {
    -      ArrayList<String> protocols = new ArrayList<>();
    -      for (TransportConfiguration acceptor : acceptors) {
    -         Object protocolsFromAcceptor = acceptor.getParams().get(TransportConstants.PROTOCOLS_PROP_NAME);
    -         if (protocolsFromAcceptor != null) {
    -            String[] protocolsSplit = protocolsFromAcceptor.toString().split(",");
    -            for (String protocol : protocolsSplit) {
    -               if (!protocols.contains(protocol)) {
    -                  protocols.add(protocol);
    -               }
    -            }
    -         }
    -      }
    -      return protocols.toArray(new String[protocols.size()]);
    -   }
    -
    -   @Deactivate
    -   public void stop() throws Exception {
    -      tracker.close();
    -   }
    -
    -   public Map<String, ActiveMQComponent> getComponents() {
    -      return components;
    -   }
    -
    -   /*
    -    * this makes sure the components are started in the correct order. Its
    -    * simple at the mo as e only have core and jms but will need impproving if
    -    * we get more.
    -    */
    -   public ArrayList<ActiveMQComponent> getComponentsByStartOrder(Map<String, ActiveMQComponent> components) {
    -      ArrayList<ActiveMQComponent> activeMQComponents = new ArrayList<>();
    -      ActiveMQComponent jmsComponent = components.get("jms");
    -      if (jmsComponent != null) {
    -         activeMQComponents.add(jmsComponent);
    -      }
    -      activeMQComponents.add(components.get("core"));
    -      return activeMQComponents;
    -   }
    -
    -   public void register(BundleContext context, Dictionary<String, ?> properties) {
    -      registrations = new HashMap<>();
    -      for (Map.Entry<String, ActiveMQComponent> component : getComponents().entrySet()) {
    -         String[] classes = getInterfaces(component.getValue());
    -         Hashtable<String, Object> props = new Hashtable<>();
    -         for (Enumeration<String> keyEnum = properties.keys(); keyEnum.hasMoreElements(); ) {
    -            String key = keyEnum.nextElement();
    -            Object val = properties.get(key);
    -            props.put(key, val);
    -         }
    -         ServiceRegistration<?> registration = context.registerService(classes, component.getValue(), props);
    -         registrations.put(component.getKey(), registration);
    -      }
    -   }
    -
    -   private String[] getInterfaces(ActiveMQComponent value) {
    -      Set<String> interfaces = new HashSet<>();
    -      getInterfaces(value.getClass(), interfaces);
    -      return interfaces.toArray(new String[interfaces.size()]);
    -   }
    -
    -   private void getInterfaces(Class<?> clazz, Set<String> interfaces) {
    -      for (Class<?> itf : clazz.getInterfaces()) {
    -         if (interfaces.add(itf.getName())) {
    -            getInterfaces(itf, interfaces);
    -         }
    -      }
    -      if (clazz.getSuperclass() != null) {
    -         getInterfaces(clazz.getSuperclass(), interfaces);
    -      }
    -   }
    -
    -   public void unregister() {
    -      if (registrations != null) {
    -         for (ServiceRegistration<?> reg : registrations.values()) {
    -            reg.unregister();
    -         }
    -      }
    -   }
    -}
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements. See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License. You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.activemq.artemis.osgi;
    +
    --- End diff --
    
    You are mixing reformat changes here. It's hard to know what you changed.
    
    
    Can you isolate only what was changed and make this a simple patch?
    
    
    
    you can then:
    
    ```sh
    git commit -a --amend
    git push origin -f
    ```


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben I thought ProtocolTracker was part of the API. it's for internal use only.
    
    We could have renamed it as much as we wanted. Since I'm the one who messed up I will fix this one :)


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @clebertsuconic great.  It looks like @graben has addressed your comments.  If you're happy I'll merge this?


---
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 #839: ARTEMIS-793 Improvement to OSGi integrat...

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/839#discussion_r83920218
  
    --- Diff: artemis-server-osgi/src/main/java/org/apache/activemq/artemis/osgi/ProtocolTrackerCallBack.java ---
    @@ -25,4 +25,6 @@
        void addFactory(ProtocolManagerFactory<Interceptor> pmf);
     
        void removeFactory(ProtocolManagerFactory<Interceptor> pmf);
    +
    +   void setDataSourceDependency(boolean dataSourceDependency);
    --- End diff --
    
    what about renaming this to:
    
    addDataSource(WhateverInformationForTheDatasource);
    
    
    the callback could then decide what to do.
    
    
    the best would be to have a class extending ProtocolTrackerCallback:
    
    
    ServerTrackerCallback extends ProtocolTrackerCallback
    
    
    And at the caller we would check an instanceof.
    
    Would that be possible with OSGI?
    
    
    
    BTW: thanks for rebasing... at least I can comment on these changes 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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben can you rebase against master... and push -f at least?


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    I love checkstyle. :) I thought about the actual implementation and did some small refactorings. Now I think it fits best. But we may think about synchronizing in callback as they are called in separate threads.


---
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 #839: ARTEMIS-793 Improvement to OSGi integrat...

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/839#discussion_r83920395
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/FileStorageConfiguration.java ---
    @@ -29,7 +29,7 @@
     
        @Override
        public StoreType getStoreType() {
    -      return StoreType.DATABASE;
    +      return StoreType.FILE;
    --- End diff --
    
    @mtaylor this was a bug, right?


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @mtaylor that merge script we always use will fix it. I already tried it  


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    Rebase is done :)


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben  ^^^ (<<< just to send you a notification)



---
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 #839: ARTEMIS-793 Improvement to OSGi integrat...

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/839#discussion_r83347252
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/config/storage/FileStorageConfiguration.java ---
    @@ -29,7 +29,7 @@
     
        @Override
        public StoreType getStoreType() {
    -      return StoreType.DATABASE;
    --- End diff --
    
    nice catch!!!!!


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben  I have merged, after I reverted some of the bad code I made you do. Can you check please?
    
    And sorry about that.. I really thought the ProtocolTracker was externally used.


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @clebertsuconic That's fine with 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.
---

[GitHub] activemq-artemis issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben I understand it works now. I was just trying to be more generic on the implementation and support future needs.
    



---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    Looks good. I give it a try this evening when back home.


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    A bit more this way? IMO I did not see why to change the method to addDataSource. It could be only one and this one is a dependency which is mandatory for broker startup.


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    sounds good


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben could you please squash you please rebase on top of master vs using the merge commit.  


---
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 #839: ARTEMIS-793 Improvement to OSGi integrat...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    @graben I actually just asked this on the dev-list


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    let me think about the class name...


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by graben <gi...@git.apache.org>.
Github user graben commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    Have to fix a bug introduced by refactoring. I think inner classes are looking better than an anonymous one. It might be better to add components to ProtocolTrackerCallBackImpl too make it a static class!?


---
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 issue #839: ARTEMIS-793 Improvement to OSGi integration

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/839
  
    There is one change that is wrong here. You are using the ProtocolTrackerCallback to admin DataSources. Can you separate this into a separate class? DataSourceTrackerCallback?
    
    And instead of returning a true or false, can you return the name of the datasources?
    
    
    
    Also: I coudln't comment on the change here given the merge mess you have on this branch now. Can  you execute these commands please?
    
    
    
    ```
    git pull --rebase upstream master
    git push origin -f your-branch-name
    ```


---
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.
---