jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Antonio Gomes Rodrigues <ra0...@gmail.com>
Subject Re: svn commit: r1736119 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
Date Fri, 25 Mar 2016 12:42:42 GMT
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 <sebbaz@gmail.com>:

> On 25 March 2016 at 11:12, Antonio Gomes Rodrigues <ra0077@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 <sebbaz@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 <sebbaz@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 <sebbaz@gmail.com>
wrote:
> >> >> >
> >> >> >> On 23 March 2016 at 15:02, Philippe Mouawad <
> >> philippe.mouawad@gmail.com
> >> >> >
> >> >> >> wrote:
> >> >> >> > I guess the clean way would have been to do something
like this
> >> (not
> >> >> >> > tested) to avoid saving the arguments that are equal
to default
> >> >> values:
> >> >> >>
> >> >> >> It's only the NEW argument that needs to be omitted if it
is the
> >> >> default.
> >> >> >>
> >> >> >
> >> >> > That's why it's a bit hacky.
> >> >>
> >> >> It's the standard way we deal with new properties.
> >> >> The defaults are not saved to the JMX file.
> >> >>
> >> >
> >> > 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 <sebbaz@gmail.com>
wrote:
> >> >> >> >
> >> >> >> >> On 22 March 2016 at 19:12, Philippe Mouawad <
> >> >> philippe.mouawad@gmail.com
> >> >> >> >
> >> >> >> >> wrote:
> >> >> >> >> > Hello sebb,
> >> >> >> >> > Although this fixes the issue, it seems to me
as a violation
> of
> >> the
> >> >> >> >> > architecture .
> >> >> >> >> > BackendListener should not be aware of a particular
> >> implementation
> >> >> of
> >> >> >> >> > BackendListenerClient : GraphiteBackendListenerClient
> >> >> >> >>
> >> >> >> >> If you can move the fix into GraphiteBackendListenerClient
> please
> >> do
> >> >> so.
> >> >> >> >>
> >> >> >> >> > Regards
> >> >> >> >> >
> >> >> >> >> > On Tue, Mar 22, 2016 at 1:54 AM, <sebb@apache.org>
wrote:
> >> >> >> >> >
> >> >> >> >> >> Author: sebb
> >> >> >> >> >> Date: Tue Mar 22 00:54:30 2016
> >> >> >> >> >> New Revision: 1736119
> >> >> >> >> >>
> >> >> >> >> >> URL: http://svn.apache.org/viewvc?rev=1736119&view=rev
> >> >> >> >> >> Log:
> >> >> >> >> >> New fields/changed defaults cause earlier
test plans to be
> >> marked
> >> >> as
> >> >> >> >> >> changed
> >> >> >> >> >> Fix BackendListener
> >> >> >> >> >> Bugzilla Id: 59173
> >> >> >> >> >>
> >> >> >> >> >> Modified:
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
> >> >> >> >> >>
> >> >> >> >> >> Modified:
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
> >> >> >> >> >> URL:
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java?rev=1736119&r1=1736118&r2=1736119&view=diff
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> ==============================================================================
> >> >> >> >> >> ---
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
> >> >> >> >> >> (original)
> >> >> >> >> >> +++
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> jmeter/trunk/src/components/org/apache/jmeter/visualizers/backend/BackendListener.java
> >> >> >> >> >> Tue Mar 22 00:54:30 2016
> >> >> >> >> >> @@ -39,6 +39,7 @@ import org.apache.jmeter.testelement.Abs
> >> >> >> >> >>  import org.apache.jmeter.testelement.TestElement;
> >> >> >> >> >>  import org.apache.jmeter.testelement.TestStateListener;
> >> >> >> >> >>  import
> >> >> org.apache.jmeter.testelement.property.TestElementProperty;
> >> >> >> >> >> +import
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> org.apache.jmeter.visualizers.backend.graphite.GraphiteBackendListenerClient;
> >> >> >> >> >>  import org.apache.jorphan.logging.LoggingManager;
> >> >> >> >> >>  import org.apache.log.Logger;
> >> >> >> >> >>
> >> >> >> >> >> @@ -434,6 +435,9 @@ public class BackendListener
extends Abs
> >> >> >> >> >>       *            the new arguments. These
replace any
> >> existing
> >> >> >> >> arguments.
> >> >> >> >> >>       */
> >> >> >> >> >>      public void setArguments(Arguments
args) {
> >> >> >> >> >> +        // Bug 59173 - don't save new default
argument
> >> >> >> >> >> +
> >> >> >> >> >>
> >> >> >> >>
> >> >> >>
> >> >>
> >>
> args.removeArgument(GraphiteBackendListenerClient.USE_REGEXP_FOR_SAMPLERS_LIST,
> >> >> >> >> >> +
> >> >> >> >> >>
> >> >> GraphiteBackendListenerClient.USE_REGEXP_FOR_SAMPLERS_LIST_DEFAULT);
> >> >> >> >> >>          setProperty(new TestElementProperty(ARGUMENTS,
> args));
> >> >> >> >> >>      }
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >> >>
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > --
> >> >> >> >> > Cordialement.
> >> >> >> >> > Philippe Mouawad.
> >> >> >> >>
> >> >> >> >
> >> >> >> >
> >> >> >> >
> >> >> >> > --
> >> >> >> > Cordialement.
> >> >> >> > Philippe Mouawad.
> >> >> >>
> >> >> >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Cordialement.
> >> >> > Philippe Mouawad.
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Cordialement.
> >> > Philippe Mouawad.
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message