jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Date Wed, 23 Mar 2016 18:41:22 GMT
On 23 March 2016 at 18:23, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> On Wed, Mar 23, 2016 at 4:16 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 23 March 2016 at 15:02, Philippe Mouawad <philippe.mouawad@gmail.com>
>> wrote:
>> > I guess the clean way would have been to do something like this (not
>> > tested) to avoid saving the arguments that are equal to default values:
>>
>> It's only the NEW argument that needs to be omitted if it is the default.
>>
>
> That's why it's a bit hacky.

It's the standard way we deal with new properties.
The defaults are not saved to the JMX file.

The only thing slightly hacky is where it is currently implemented.
A better solution is awaited.
Please help to find that rather than arguing it cannot be done.

> We could introduce a new method getArgumentsNotToSaveIfEqualToDefault() :-)
> in interface but I find it strange

There has to be another way that is cleaner.

>>
>> Note that the code already adds the new argument if it is not present
>> in the JMX.
>> The code just needs to omit it when writing the JMX.
>> This is symmetrical as far as that argument is concerned.
>> The fact that the other arguments are always saved in the JMX is
>> arguably a bug in the orginal release, but we definitely cannot change
>> that now.
>>
>> I found a way of doing it that is not ideal, but at least it works.
>>
>> >     public void setArguments(Arguments args) {
>> >         try {
>> >             BackendListenerClient client = (BackendListenerClient)
>> > Class.forName(getClassname(), true,
>> >
>> > Thread.currentThread().getContextClassLoader()).newInstance();
>> >             Arguments defaultArgs = client.getDefaultParameters();
>> >             PropertyIterator iter = defaultArgs.iterator();
>> >             while (iter.hasNext()) {
>> >                 Argument defaultArg = (Argument)
>> > iter.next().getObjectValue();
>> >                 args.removeArgument(defaultArg.getName(),
>> > defaultArg.getValue());
>> >             }
>> >         } catch (InstantiationException | IllegalAccessException|
>> > ClassNotFoundException e) {
>> >         }
>> >
>> >         setProperty(new TestElementProperty(ARGUMENTS, args));
>> >     }
>> >
>> >
>> > But doing this now is too late
>>
>> It's not too late.
>>
>
> Why not make it clean and accept the minor issue to tell user his plan has
> changed.

Because this will keep happening unless we find a solution.

> This way it will be fixed for next version if we enhance component of it
> new client implementation are introduced.

No idea what that sentence means.

> Note we have same issue with AbstractJavaSamplerClient but it's less
> impacting as we don't provide any that's feature rich.
>
>
>>
>> > or we could do it and accept that plan changes on load.
>>
>> > But doing it only for the new field looks strange if not ugly to me and
>>
>> > anyway I don't think it should be done here as client can be of any class
>> > not just GraphiteBackendListenerClient
>>
>> Agreed it would be better to do it only for that client.
>>
>> Suggestions welcome.
>>
>> > Regards
>> > Philippe
>> >
>> > On Tue, Mar 22, 2016 at 9:09 PM, sebb <sebbaz@gmail.com> wrote:
>> >
>> >> On 22 March 2016 at 19:12, Philippe Mouawad <philippe.mouawad@gmail.com
>> >
>> >> wrote:
>> >> > Hello sebb,
>> >> > Although this fixes the issue, it seems to me as a violation of the
>> >> > architecture .
>> >> > BackendListener should not be aware of a particular implementation
of
>> >> > BackendListenerClient : GraphiteBackendListenerClient
>> >>
>> >> If you can move the fix into GraphiteBackendListenerClient please do so.
>> >>
>> >> > Regards
>> >> >
>> >> > On Tue, Mar 22, 2016 at 1:54 AM, <sebb@apache.org> wrote:
>> >> >
>> >> >> Author: sebb
>> >> >> Date: Tue Mar 22 00:54:30 2016
>> >> >> New Revision: 1736119
>> >> >>
>> >> >> URL: http://svn.apache.org/viewvc?rev=1736119&view=rev
>> >> >> Log:
>> >> >> New fields/changed defaults cause earlier test plans to be marked
as
>> >> >> changed
>> >> >> Fix BackendListener
>> >> >> Bugzilla Id: 59173
>> >> >>
>> >> >> Modified:
>> >> >>
>> >> >>
>> >>
>> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
>> >> >>
>> >> >> Modified:
>> >> >>
>> >>
>> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
>> >> >> URL:
>> >> >>
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java?rev=1736119&r1=1736118&r2=1736119&view=diff
>> >> >>
>> >> >>
>> >>
>> ==============================================================================
>> >> >> ---
>> >> >>
>> >>
>> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
>> >> >> (original)
>> >> >> +++
>> >> >>
>> >>
>> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
>> >> >> Tue Mar 22 00:54:30 2016
>> >> >> @@ -39,6 +39,7 @@ import org.apache.jmeter.testelement.Abs
>> >> >>  import org.apache.jmeter.testelement.TestElement;
>> >> >>  import org.apache.jmeter.testelement.TestStateListener;
>> >> >>  import org.apache.jmeter.testelement.property.TestElementProperty;
>> >> >> +import
>> >> >>
>> >>
>> org.apache.jmeter.visualizers.backend.graphite.GraphiteBackendListenerClient;
>> >> >>  import org.apache.jorphan.logging.LoggingManager;
>> >> >>  import org.apache.log.Logger;
>> >> >>
>> >> >> @@ -434,6 +435,9 @@ public class BackendListener extends Abs
>> >> >>       *            the new arguments. These replace any existing
>> >> arguments.
>> >> >>       */
>> >> >>      public void setArguments(Arguments args) {
>> >> >> +        // Bug 59173 - don't save new default argument
>> >> >> +
>> >> >>
>> >>
>> args.removeArgument(GraphiteBackendListenerClient.USE_REGEXP_FOR_SAMPLERS_LIST,
>> >> >> +
>> >> >> GraphiteBackendListenerClient.USE_REGEXP_FOR_SAMPLERS_LIST_DEFAULT);
>> >> >>          setProperty(new TestElementProperty(ARGUMENTS, args));
>> >> >>      }
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >
>> >> >
>> >> > --
>> >> > Cordialement.
>> >> > Philippe Mouawad.
>> >>
>> >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message