On Wed, Mar 23, 2016 at 7:41 PM, sebb <seb...@gmail.com> wrote:
> On 23 March 2016 at 18:23, Philippe Mouawad <philippe.moua...@gmail.com>
> wrote:
> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <seb...@gmail.com> wrote:
> >
> >> On 23 March 2016 at 15:02, Philippe Mouawad <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> wrote:
> >> >
> >> >> On 22 March 2016 at 19:12, Philippe Mouawad <
> 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> 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.
