[GitHub] activemq-artemis pull request #1737: NO-JIRA fixing master

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1737: NO-JIRA fixing master

franz1981
GitHub user michaelandrepearce opened a pull request:

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

    NO-JIRA fixing master

   

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

    $ git pull https://github.com/michaelandrepearce/activemq-artemis FIXMASTER

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

    https://github.com/apache/activemq-artemis/pull/1737.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 #1737
   
----
commit dd67f855cda22d0098afe262c0f2bf7098c6e230
Author: Michael André Pearce <michael.andre.pearce@...>
Date:   2017-12-21T07:08:44Z

    NO-JIRA fixing master

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1737: NO-JIRA fixing master

franz1981
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1737
 
    This is a fix, as PR build is failing ever since https://github.com/apache/activemq-artemis/pull/1683 got merged.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1737: NO-JIRA fixing master

franz1981
In reply to this post by franz1981
Github user asfgit closed the pull request at:

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


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1737: NO-JIRA fixing master

franz1981
In reply to this post by franz1981
Github user jdanekrh commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1737
 
    The test was broken in 2013 by @jbertram 's commit https://github.com/hornetq/hornetq/pull/1879 for Jira https://issues.jboss.org/browse/HORNETQ-1411.
   
    The relevant diff is (abbreviated)
   
    ```diff
    -   private final long timeout;
    +   private long timeout = HornetQDefaultConfiguration.getDefaultGroupingHandlerTimeout();
    [...]
    +   public GroupingHandlerConfiguration()
        {
    -      this.type = type;
    -      this.name = name;
    -      this.address = address;
    -      this.timeout = timeout;
    -      if (System.getProperty(GROUP_TIMEOUT_PROP_NAME) != null)
    -      {
    -         this.groupTimeout = Long.parseLong(System.getProperty(GROUP_TIMEOUT_PROP_NAME));
    -      }
    -      else
    -      {
    -         this.groupTimeout = groupTimeout;
    -      }
    -
    -      if (System.getProperty(REAPER_PERIOD_PROP_NAME) != null)
    -      {
    -         this.reaperPeriod = Long.parseLong(System.getProperty(REAPER_PERIOD_PROP_NAME));
    -      }
    -      else
    -      {
    -         this.reaperPeriod = reaperPeriod;
    -      }
        }
    ```


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1737: NO-JIRA fixing master

franz1981
In reply to this post by franz1981
Github user jdanekrh commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1737
 
    The test seems to have been never running (never was annotated with `@Test`, maybe previous test runner did not require it?) and nobody complained when it was broken so long ago, apparently.
   
    I propose deleting the `SystemPropertyOverrideTest#testSystemPropertyOverride` test.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1737: NO-JIRA fixing master

franz1981
In reply to this post by franz1981
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1737
 
    @jdanekrh Clebert later fixed and re-enabled.  


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1737: NO-JIRA fixing master

franz1981
In reply to this post by franz1981
Github user jdanekrh commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1737
 
    Thanks, I've pulled most recent changes and now I see the fix in place.
   
    Weird that there are some special parameters that can be configured through system properties, while most cannot... I guess somebody had reason when asking for it.


---