You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by Philippe Mouawad <ph...@gmail.com> on 2016/03/22 20:12:49 UTC
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
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
Regards
On Tue, Mar 22, 2016 at 1:54 AM, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Philippe Mouawad <ph...@gmail.com>.
I created a bug to track this:
https://bz.apache.org/bugzilla/show_bug.cgi?id=59241
I closed 59173 <https://bz.apache.org/bugzilla/show_bug.cgi?id=59173>
Regards
On Fri, Mar 25, 2016 at 1:42 PM, Antonio Gomes Rodrigues <ra...@gmail.com>
wrote:
> Hi Sebb,
>
> Thanks to your quick answer, now it's more cleare to me
>
> If file size is a problem, I don't think to keep xml format is a good idea
>
> Antonio
>
>
> 2016-03-25 13:26 GMT+01:00 sebb <se...@gmail.com>:
>
> > On 25 March 2016 at 11:12, Antonio Gomes Rodrigues <ra...@gmail.com>
> > wrote:
> > > Hi,
> > >
> > > Why not define a new JMX output to 3.0?
> > >
> > > If the new format save all the value (default or not), is that problem
> > > would be solved and never appear?
> >
> > No, because every time a new property is added, the JMX would change.
> > That is the main point of not saving the default value for new
> properties.
> >
> > (A secondary effect is not increasing the JMX file size).
> >
> > For people that keep test plans in a CMS, it's a nuisance if
> > unnecessary data is added between releases.
> > Makes it harder to tell what has really changed.
> >
> > > If yes, why don't have the 2 solutions for 3.0 (Save old format + Save
> > new
> > > format) and deprecated old format some release later?
> > >
> > > Antonio
> > >
> > > 2016-03-23 23:23 GMT+01:00 sebb <se...@gmail.com>:
> > >
> > >> On 23 March 2016 at 19:31, Philippe Mouawad <
> philippe.mouawad@gmail.com
> > >
> > >> wrote:
> > >> > On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
> > >> >
> > >> >> On 23 March 2016 at 18:23, Philippe Mouawad <
> > philippe.mouawad@gmail.com
> > >> >
> > >> >> wrote:
> > >> >> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
> > >> >>
> > >> >
> > >> > 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.
> > >>
> > >> I see now.
> > >>
> > >> So you want to break the JMX compatibility once, and then not do so
> > >> again, even if more properties are added?
> > >>
> > >> That is another approach, but I prefer not to break compatibility at
> > all.
> > >> I regard compatibility as much more important than clean code
> > >> (whatever that may mean).
> > >>
> > >> But at least there seems to be agreement not to cause unnecessary
> > >> changes to the JMX file between versions.
> > >>
> > >> >
> > >> >> > 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 <se...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi Sebb,
Thanks to your quick answer, now it's more cleare to me
If file size is a problem, I don't think to keep xml format is a good idea
Antonio
2016-03-25 13:26 GMT+01:00 sebb <se...@gmail.com>:
> On 25 March 2016 at 11:12, Antonio Gomes Rodrigues <ra...@gmail.com>
> wrote:
> > Hi,
> >
> > Why not define a new JMX output to 3.0?
> >
> > If the new format save all the value (default or not), is that problem
> > would be solved and never appear?
>
> No, because every time a new property is added, the JMX would change.
> That is the main point of not saving the default value for new properties.
>
> (A secondary effect is not increasing the JMX file size).
>
> For people that keep test plans in a CMS, it's a nuisance if
> unnecessary data is added between releases.
> Makes it harder to tell what has really changed.
>
> > If yes, why don't have the 2 solutions for 3.0 (Save old format + Save
> new
> > format) and deprecated old format some release later?
> >
> > Antonio
> >
> > 2016-03-23 23:23 GMT+01:00 sebb <se...@gmail.com>:
> >
> >> On 23 March 2016 at 19:31, Philippe Mouawad <philippe.mouawad@gmail.com
> >
> >> wrote:
> >> > On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
> >> >
> >> >> On 23 March 2016 at 18:23, Philippe Mouawad <
> philippe.mouawad@gmail.com
> >> >
> >> >> wrote:
> >> >> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
> >> >>
> >> >
> >> > 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.
> >>
> >> I see now.
> >>
> >> So you want to break the JMX compatibility once, and then not do so
> >> again, even if more properties are added?
> >>
> >> That is another approach, but I prefer not to break compatibility at
> all.
> >> I regard compatibility as much more important than clean code
> >> (whatever that may mean).
> >>
> >> But at least there seems to be agreement not to cause unnecessary
> >> changes to the JMX file between versions.
> >>
> >> >
> >> >> > 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 <se...@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, <se...@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.
> >>
>
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by sebb <se...@gmail.com>.
On 25 March 2016 at 11:12, Antonio Gomes Rodrigues <ra...@gmail.com> wrote:
> Hi,
>
> Why not define a new JMX output to 3.0?
>
> If the new format save all the value (default or not), is that problem
> would be solved and never appear?
No, because every time a new property is added, the JMX would change.
That is the main point of not saving the default value for new properties.
(A secondary effect is not increasing the JMX file size).
For people that keep test plans in a CMS, it's a nuisance if
unnecessary data is added between releases.
Makes it harder to tell what has really changed.
> If yes, why don't have the 2 solutions for 3.0 (Save old format + Save new
> format) and deprecated old format some release later?
>
> Antonio
>
> 2016-03-23 23:23 GMT+01:00 sebb <se...@gmail.com>:
>
>> On 23 March 2016 at 19:31, Philippe Mouawad <ph...@gmail.com>
>> wrote:
>> > On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
>> >
>> >> On 23 March 2016 at 18:23, Philippe Mouawad <philippe.mouawad@gmail.com
>> >
>> >> wrote:
>> >> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
>> >>
>> >
>> > 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.
>>
>> I see now.
>>
>> So you want to break the JMX compatibility once, and then not do so
>> again, even if more properties are added?
>>
>> That is another approach, but I prefer not to break compatibility at all.
>> I regard compatibility as much more important than clean code
>> (whatever that may mean).
>>
>> But at least there seems to be agreement not to cause unnecessary
>> changes to the JMX file between versions.
>>
>> >
>> >> > 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 <se...@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, <se...@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.
>>
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Antonio Gomes Rodrigues <ra...@gmail.com>.
Hi,
Why not define a new JMX output to 3.0?
If the new format save all the value (default or not), is that problem
would be solved and never appear?
If yes, why don't have the 2 solutions for 3.0 (Save old format + Save new
format) and deprecated old format some release later?
Antonio
2016-03-23 23:23 GMT+01:00 sebb <se...@gmail.com>:
> On 23 March 2016 at 19:31, Philippe Mouawad <ph...@gmail.com>
> wrote:
> > On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
> >
> >> On 23 March 2016 at 18:23, Philippe Mouawad <philippe.mouawad@gmail.com
> >
> >> wrote:
> >> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
> >>
> >
> > 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.
>
> I see now.
>
> So you want to break the JMX compatibility once, and then not do so
> again, even if more properties are added?
>
> That is another approach, but I prefer not to break compatibility at all.
> I regard compatibility as much more important than clean code
> (whatever that may mean).
>
> But at least there seems to be agreement not to cause unnecessary
> changes to the JMX file between versions.
>
> >
> >> > 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 <se...@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, <se...@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.
>
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by sebb <se...@gmail.com>.
On 23 March 2016 at 19:31, Philippe Mouawad <ph...@gmail.com> wrote:
> On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
>
>> On 23 March 2016 at 18:23, Philippe Mouawad <ph...@gmail.com>
>> wrote:
>> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
>>
>
> 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.
I see now.
So you want to break the JMX compatibility once, and then not do so
again, even if more properties are added?
That is another approach, but I prefer not to break compatibility at all.
I regard compatibility as much more important than clean code
(whatever that may mean).
But at least there seems to be agreement not to cause unnecessary
changes to the JMX file between versions.
>
>> > 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 <se...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Philippe Mouawad <ph...@gmail.com>.
and anyway, thanks for fixing the 3 other issues that I thought would be
tougher to fix
On Wednesday, March 23, 2016, Philippe Mouawad <ph...@gmail.com>
wrote:
>
>
> On Wed, Mar 23, 2016 at 7:41 PM, sebb <sebbaz@gmail.com
> <javascript:_e(%7B%7D,'cvml','sebbaz@gmail.com');>> wrote:
>
>> On 23 March 2016 at 18:23, Philippe Mouawad <philippe.mouawad@gmail.com
>> <javascript:_e(%7B%7D,'cvml','philippe.mouawad@gmail.com');>> wrote:
>> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <sebbaz@gmail.com
>> <javascript:_e(%7B%7D,'cvml','sebbaz@gmail.com');>> wrote:
>> >
>> >> On 23 March 2016 at 15:02, Philippe Mouawad <
>> philippe.mouawad@gmail.com
>> <javascript:_e(%7B%7D,'cvml','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.
>>
>
> 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 <sebbaz@gmail.com
>> <javascript:_e(%7B%7D,'cvml','sebbaz@gmail.com');>> wrote:
>> >> >
>> >> >> On 22 March 2016 at 19:12, Philippe Mouawad <
>> philippe.mouawad@gmail.com
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>
--
Cordialement.
Philippe Mouawad.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Philippe Mouawad <ph...@gmail.com>.
On Wed, Mar 23, 2016 at 7:41 PM, sebb <se...@gmail.com> wrote:
> On 23 March 2016 at 18:23, Philippe Mouawad <ph...@gmail.com>
> wrote:
> > On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@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.
>
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 <se...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by sebb <se...@gmail.com>.
On 23 March 2016 at 18:23, Philippe Mouawad <ph...@gmail.com> wrote:
> On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@gmail.com> wrote:
>
>> On 23 March 2016 at 15:02, Philippe Mouawad <ph...@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 <se...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Philippe Mouawad <ph...@gmail.com>.
On Wed, Mar 23, 2016 at 4:16 PM, sebb <se...@gmail.com> wrote:
> On 23 March 2016 at 15:02, Philippe Mouawad <ph...@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.
We could introduce a new method getArgumentsNotToSaveIfEqualToDefault() :-)
in interface but I find it strange
>
> 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.
This way it will be fixed for next version if we enhance component of it
new client implementation are introduced.
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 <se...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by sebb <se...@gmail.com>.
On 23 March 2016 at 15:02, Philippe Mouawad <ph...@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.
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.
> 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 <se...@gmail.com> wrote:
>
>> On 22 March 2016 at 19:12, Philippe Mouawad <ph...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by Philippe Mouawad <ph...@gmail.com>.
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:
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 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
Regards
Philippe
On Tue, Mar 22, 2016 at 9:09 PM, sebb <se...@gmail.com> wrote:
> On 22 March 2016 at 19:12, Philippe Mouawad <ph...@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, <se...@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.
Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Posted by sebb <se...@gmail.com>.
On 22 March 2016 at 19:12, Philippe Mouawad <ph...@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, <se...@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.