and anyway, thanks for fixing the 3 other issues that I thought would be
tougher to fix
On Wednesday, March 23, 2016, Philippe Mouawad <philippe.moua...@gmail.com>
wrote:
>
>
> On Wed, Mar 23, 2016 at 7:41 PM, sebb <seb...@gmail.com
> <javascript:_e(%7B%7D,'cvml','seb...@gmail.com');>> wrote:
>
>> On 23 March 2016 at 18:23, Philippe Mouawad <philippe.moua...@gmail.com
>> <javascript:_e(%7B%7D,'cvml','philippe.moua...@gmail.com');>> wrote:
>> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <seb...@gmail.com
>> <javascript:_e(%7B%7D,'cvml','seb...@gmail.com');>> wrote:
>> >
>> >> On 23 March 2016 at 15:02, Philippe Mouawad <
>> philippe.moua...@gmail.com
>> <javascript:_e(%7B%7D,'cvml','philippe.moua...@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.
>>
>
> Thank you , I am aware of that ...
> What's hacky here is "why only 1 property of all other arguments is not
> saved".
>
>>
>> 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.
>>
>
> Isn't it what I am doing just 1 sentence after this one ? Even if am
> joking (you don't taste it I know) about my own proposal.
> Try to joke sometimes, it will be funnier than being bad tempered ...
>
>
>> > We could introduce a new method getArgumentsNotToSaveIfEqualToDefault()
>> :-)
>> > in interface but I find it strange
>>
>> There has to be another way that is cleaner.
>>
>
> I'll let you find it...
>
>>
>> >>
>> >> 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.
>>
> It will for people loading a < 3.0.
> Once we switch to 3.1 and superior, it will be fixed
>
>>
>> > 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.
>>
>
> What I call clean is not to save the properties/arguments that are equal
> to defaults.
> So it will introduce this annoyance for script loaded from < 3.0, but one
> fixed it will be for scripts loaded from 3.0 and above.
>
>
>> > 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 <seb...@gmail.com
>> <javascript:_e(%7B%7D,'cvml','seb...@gmail.com');>> wrote:
>> >> >
>> >> >> On 22 March 2016 at 19:12, Philippe Mouawad <
>> philippe.moua...@gmail.com
>> <javascript:_e(%7B%7D,'cvml','philippe.moua...@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, <s...@apache.org
>> <javascript:_e(%7B%7D,'cvml','s...@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.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>
--
Cordialement.
Philippe Mouawad.
