[bisect] - Add --path_to_config parameter to run script.

Add --path_to_config parameter to run script. This is needed if the config is located in another depot.

BUG=301552
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263136

[prasadv, 2014-04-09 23:17:50]

lgtm

https://codereview.chromium.org/232083003/diff/1/tools/run-bisect-perf-regres...
File tools/run-bisect-perf-regression.py (right):

https://codereview.chromium.org/232083003/diff/1/tools/run-bisect-perf-regres...
tools/run-bisect-perf-regression.py:392: path_to_bisect_cfg =
opts.path_to_config
Should'n we validate whether the path_to_config is file and if it exists? or the
format of the content?

[shatch, 2014-04-10 17:32:53]

Added some config validation, ptal.

https://codereview.chromium.org/232083003/diff/1/tools/run-bisect-perf-regres...
File tools/run-bisect-perf-regression.py (right):

https://codereview.chromium.org/232083003/diff/1/tools/run-bisect-perf-regres...
tools/run-bisect-perf-regression.py:392: path_to_bisect_cfg =
opts.path_to_config
On 2014/04/09 23:17:51, prasadv wrote:
> Should'n we validate whether the path_to_config is file and if it exists? or
the
> format of the content?

Done.

[qyearsley, 2014-04-10 17:58:22]

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
File tools/run-bisect-perf-regression.py (right):

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:119: """Validates that the config file
contents is a valid perf config, and all
Nit: This wording is a little bit redundant, since "to validate" means "to check
whether sth is valid".

Maybe something like: "Validates the config file contents, checking whether all
values are non-empty."

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:132: config_has_values = [v for v in
config_contents.values() if v]
What if the value used for truncate_percent was 0?

It looks like we're expecting that all of the values in the config dict must be
non-empty strings.

If you think that later someone might want to directly put integers in the
config file, maybe here you could check  
[v != '' and v is not None for v in config_values.values() if v]

OR, if we want to assert that the values in the config must be strings, perhaps
you could use:

[v and type(v) is str for v in config_values.values() if v]

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:139: """Validates that the config file
contents is a valid perf config.
I wasn't aware before that there was such a thing as a "perf config" (usually at
run-perf-test.cfg, right?)

Maybe add a note saying that a "perf config file" is a config used when we only
want to run one perf test once, and we're not actually bisecting; it's usually
called run-perf-test.cfg by default.

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
opts.path_to_config
At this point, we're not 100% sure whether they've supplied a "bisect config" or
"perf config".

[shatch, 2014-04-10 18:21:11]

New snapshot uploaded, ptal.

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
File tools/run-bisect-perf-regression.py (right):

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:119: """Validates that the config file
contents is a valid perf config, and all
On 2014/04/10 17:58:22, qyearsley wrote:
> Nit: This wording is a little bit redundant, since "to validate" means "to
check
> whether sth is valid".
> 
> Maybe something like: "Validates the config file contents, checking whether
all
> values are non-empty."

Done.

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:132: config_has_values = [v for v in
config_contents.values() if v]
On 2014/04/10 17:58:22, qyearsley wrote:
> What if the value used for truncate_percent was 0?
> 
> It looks like we're expecting that all of the values in the config dict must
be
> non-empty strings.
> 
> If you think that later someone might want to directly put integers in the
> config file, maybe here you could check  
> [v != '' and v is not None for v in config_values.values() if v]
> 
> OR, if we want to assert that the values in the config must be strings,
perhaps
> you could use:
> 
> [v and type(v) is str for v in config_values.values() if v]

Yeah, they've traditionally just been strings. Added the extra string check.

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:139: """Validates that the config file
contents is a valid perf config.
On 2014/04/10 17:58:22, qyearsley wrote:
> I wasn't aware before that there was such a thing as a "perf config" (usually
at
> run-perf-test.cfg, right?)
> 
> Maybe add a note saying that a "perf config file" is a config used when we
only
> want to run one perf test once, and we're not actually bisecting; it's usually
> called run-perf-test.cfg by default.

Done.

https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
opts.path_to_config
On 2014/04/10 17:58:22, qyearsley wrote:
> At this point, we're not 100% sure whether they've supplied a "bisect config"
or
> "perf config".

True, added some changes so that if loading it as a bisect config fails, it
falls back to treating it as a perf config.

[qyearsley, 2014-04-10 18:30:24]

On 2014/04/10 18:21:11, shatch wrote:
> New snapshot uploaded, ptal.
> 
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> File tools/run-bisect-perf-regression.py (right):
> 
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> tools/run-bisect-perf-regression.py:119: """Validates that the config file
> contents is a valid perf config, and all
> On 2014/04/10 17:58:22, qyearsley wrote:
> > Nit: This wording is a little bit redundant, since "to validate" means "to
> check
> > whether sth is valid".
> > 
> > Maybe something like: "Validates the config file contents, checking whether
> all
> > values are non-empty."
> 
> Done.
> 
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> tools/run-bisect-perf-regression.py:132: config_has_values = [v for v in
> config_contents.values() if v]
> On 2014/04/10 17:58:22, qyearsley wrote:
> > What if the value used for truncate_percent was 0?
> > 
> > It looks like we're expecting that all of the values in the config dict must
> be
> > non-empty strings.
> > 
> > If you think that later someone might want to directly put integers in the
> > config file, maybe here you could check  
> > [v != '' and v is not None for v in config_values.values() if v]
> > 
> > OR, if we want to assert that the values in the config must be strings,
> perhaps
> > you could use:
> > 
> > [v and type(v) is str for v in config_values.values() if v]
> 
> Yeah, they've traditionally just been strings. Added the extra string check.
> 
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> tools/run-bisect-perf-regression.py:139: """Validates that the config file
> contents is a valid perf config.
> On 2014/04/10 17:58:22, qyearsley wrote:
> > I wasn't aware before that there was such a thing as a "perf config"
(usually
> at
> > run-perf-test.cfg, right?)
> > 
> > Maybe add a note saying that a "perf config file" is a config used when we
> only
> > want to run one perf test once, and we're not actually bisecting; it's
usually
> > called run-perf-test.cfg by default.
> 
> Done.
> 
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
> opts.path_to_config
> On 2014/04/10 17:58:22, qyearsley wrote:
> > At this point, we're not 100% sure whether they've supplied a "bisect
config"
> or
> > "perf config".
> 
> True, added some changes so that if loading it as a bisect config fails, it
> falls back to treating it as a perf config.

LGTM.

Note: Soon I think we're going to want to add additional optional parameters to
the bisect config file, containing information about things like where to find
the archived builds.

In light of this, you could also note (if you think it's worth it) that the
parameter names which are checked are the required parameters; there could be
additional optional parameters and the validation check would still pass.

[shatch, 2014-04-10 18:36:15]

On 2014/04/10 18:30:24, qyearsley wrote:
> On 2014/04/10 18:21:11, shatch wrote:
> > New snapshot uploaded, ptal.
> > 
> >
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> > File tools/run-bisect-perf-regression.py (right):
> > 
> >
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> > tools/run-bisect-perf-regression.py:119: """Validates that the config file
> > contents is a valid perf config, and all
> > On 2014/04/10 17:58:22, qyearsley wrote:
> > > Nit: This wording is a little bit redundant, since "to validate" means "to
> > check
> > > whether sth is valid".
> > > 
> > > Maybe something like: "Validates the config file contents, checking
whether
> > all
> > > values are non-empty."
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> > tools/run-bisect-perf-regression.py:132: config_has_values = [v for v in
> > config_contents.values() if v]
> > On 2014/04/10 17:58:22, qyearsley wrote:
> > > What if the value used for truncate_percent was 0?
> > > 
> > > It looks like we're expecting that all of the values in the config dict
must
> > be
> > > non-empty strings.
> > > 
> > > If you think that later someone might want to directly put integers in the
> > > config file, maybe here you could check  
> > > [v != '' and v is not None for v in config_values.values() if v]
> > > 
> > > OR, if we want to assert that the values in the config must be strings,
> > perhaps
> > > you could use:
> > > 
> > > [v and type(v) is str for v in config_values.values() if v]
> > 
> > Yeah, they've traditionally just been strings. Added the extra string check.
> > 
> >
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> > tools/run-bisect-perf-regression.py:139: """Validates that the config file
> > contents is a valid perf config.
> > On 2014/04/10 17:58:22, qyearsley wrote:
> > > I wasn't aware before that there was such a thing as a "perf config"
> (usually
> > at
> > > run-perf-test.cfg, right?)
> > > 
> > > Maybe add a note saying that a "perf config file" is a config used when we
> > only
> > > want to run one perf test once, and we're not actually bisecting; it's
> usually
> > > called run-perf-test.cfg by default.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/232083003/diff/20001/tools/run-bisect-perf-re...
> > tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
> > opts.path_to_config
> > On 2014/04/10 17:58:22, qyearsley wrote:
> > > At this point, we're not 100% sure whether they've supplied a "bisect
> config"
> > or
> > > "perf config".
> > 
> > True, added some changes so that if loading it as a bisect config fails, it
> > falls back to treating it as a perf config.
> 
> LGTM.
> 
> Note: Soon I think we're going to want to add additional optional parameters
to
> the bisect config file, containing information about things like where to find
> the archived builds.
> 
> In light of this, you could also note (if you think it's worth it) that the
> parameter names which are checked are the required parameters; there could be
> additional optional parameters and the validation check would still pass.

Are these new parameters going to be in the config file itself, or passed as
parameters to run-bisect-perf-regression.py? Seems like it would be the latter.

[prasadv1, 2014-04-10 18:49:07]

I think these should be a part of config file itself.
Since run-bisect-perf-regression.py is called in one of the build step on
bisect bot, we might not have control on whether to pass parameters to
download archive.
Myself and Quinten discussed and felt that modifying config data on demand
from the dashboard should give more control whether to download build or
not.

Let us know if this should be the case.


On Thu, Apr 10, 2014 at 11:36 AM, <simonhatch@chromium.org> wrote:

> On 2014/04/10 18:30:24, qyearsley wrote:
>
>> On 2014/04/10 18:21:11, shatch wrote:
>> > New snapshot uploaded, ptal.
>> >
>> >
>>
>
> https://codereview.chromium.org/232083003/diff/20001/
> tools/run-bisect-perf-regression.py
>
>> > File tools/run-bisect-perf-regression.py (right):
>> >
>> >
>>
>
> https://codereview.chromium.org/232083003/diff/20001/
> tools/run-bisect-perf-regression.py#newcode119
>
>> > tools/run-bisect-perf-regression.py:119: """Validates that the config
>> file
>> > contents is a valid perf config, and all
>> > On 2014/04/10 17:58:22, qyearsley wrote:
>> > > Nit: This wording is a little bit redundant, since "to validate"
>> means "to
>> > check
>> > > whether sth is valid".
>> > >
>> > > Maybe something like: "Validates the config file contents, checking
>>
> whether
>
>> > all
>> > > values are non-empty."
>> >
>> > Done.
>> >
>> >
>>
>
> https://codereview.chromium.org/232083003/diff/20001/
> tools/run-bisect-perf-regression.py#newcode132
>
>> > tools/run-bisect-perf-regression.py:132: config_has_values = [v for v
>> in
>> > config_contents.values() if v]
>> > On 2014/04/10 17:58:22, qyearsley wrote:
>> > > What if the value used for truncate_percent was 0?
>> > >
>> > > It looks like we're expecting that all of the values in the config
>> dict
>>
> must
>
>> > be
>> > > non-empty strings.
>> > >
>> > > If you think that later someone might want to directly put integers
>> in the
>> > > config file, maybe here you could check
>> > > [v != '' and v is not None for v in config_values.values() if v]
>> > >
>> > > OR, if we want to assert that the values in the config must be
>> strings,
>> > perhaps
>> > > you could use:
>> > >
>> > > [v and type(v) is str for v in config_values.values() if v]
>> >
>> > Yeah, they've traditionally just been strings. Added the extra string
>> check.
>> >
>> >
>>
>
> https://codereview.chromium.org/232083003/diff/20001/
> tools/run-bisect-perf-regression.py#newcode139
>
>> > tools/run-bisect-perf-regression.py:139: """Validates that the config
>> file
>> > contents is a valid perf config.
>> > On 2014/04/10 17:58:22, qyearsley wrote:
>> > > I wasn't aware before that there was such a thing as a "perf config"
>> (usually
>> > at
>> > > run-perf-test.cfg, right?)
>> > >
>> > > Maybe add a note saying that a "perf config file" is a config used
>> when we
>> > only
>> > > want to run one perf test once, and we're not actually bisecting; it's
>> usually
>> > > called run-perf-test.cfg by default.
>> >
>> > Done.
>> >
>> >
>>
>
> https://codereview.chromium.org/232083003/diff/20001/
> tools/run-bisect-perf-regression.py#newcode440
>
>> > tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
>> > opts.path_to_config
>> > On 2014/04/10 17:58:22, qyearsley wrote:
>> > > At this point, we're not 100% sure whether they've supplied a "bisect
>> config"
>> > or
>> > > "perf config".
>> >
>> > True, added some changes so that if loading it as a bisect config
>> fails, it
>> > falls back to treating it as a perf config.
>>
>
>  LGTM.
>>
>
>  Note: Soon I think we're going to want to add additional optional
>> parameters
>>
> to
>
>> the bisect config file, containing information about things like where to
>> find
>> the archived builds.
>>
>
>  In light of this, you could also note (if you think it's worth it) that
>> the
>> parameter names which are checked are the required parameters; there
>> could be
>> additional optional parameters and the validation check would still pass.
>>
>
> Are these new parameters going to be in the config file itself, or passed
> as
> parameters to run-bisect-perf-regression.py? Seems like it would be the
> latter.
>
> https://codereview.chromium.org/232083003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[prasadv1, 2014-04-10 18:51:30]

correction!
Let us know if this shouldn't be the case.


On Thu, Apr 10, 2014 at 11:49 AM, Prasad Vuppalapu <prasadv@google.com>wrote:

> I think these should be a part of config file itself.
> Since run-bisect-perf-regression.py is called in one of the build step on
> bisect bot, we might not have control on whether to pass parameters to
> download archive.
> Myself and Quinten discussed and felt that modifying config data on demand
> from the dashboard should give more control whether to download build or
> not.
>
> Let us know if this should be the case.
>
>
> On Thu, Apr 10, 2014 at 11:36 AM, <simonhatch@chromium.org> wrote:
>
>> On 2014/04/10 18:30:24, qyearsley wrote:
>>
>>> On 2014/04/10 18:21:11, shatch wrote:
>>> > New snapshot uploaded, ptal.
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.org/232083003/diff/20001/
>> tools/run-bisect-perf-regression.py
>>
>>> > File tools/run-bisect-perf-regression.py (right):
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.org/232083003/diff/20001/
>> tools/run-bisect-perf-regression.py#newcode119
>>
>>> > tools/run-bisect-perf-regression.py:119: """Validates that the config
>>> file
>>> > contents is a valid perf config, and all
>>> > On 2014/04/10 17:58:22, qyearsley wrote:
>>> > > Nit: This wording is a little bit redundant, since "to validate"
>>> means "to
>>> > check
>>> > > whether sth is valid".
>>> > >
>>> > > Maybe something like: "Validates the config file contents, checking
>>>
>> whether
>>
>>> > all
>>> > > values are non-empty."
>>> >
>>> > Done.
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.org/232083003/diff/20001/
>> tools/run-bisect-perf-regression.py#newcode132
>>
>>> > tools/run-bisect-perf-regression.py:132: config_has_values = [v for v
>>> in
>>> > config_contents.values() if v]
>>> > On 2014/04/10 17:58:22, qyearsley wrote:
>>> > > What if the value used for truncate_percent was 0?
>>> > >
>>> > > It looks like we're expecting that all of the values in the config
>>> dict
>>>
>> must
>>
>>> > be
>>> > > non-empty strings.
>>> > >
>>> > > If you think that later someone might want to directly put integers
>>> in the
>>> > > config file, maybe here you could check
>>> > > [v != '' and v is not None for v in config_values.values() if v]
>>> > >
>>> > > OR, if we want to assert that the values in the config must be
>>> strings,
>>> > perhaps
>>> > > you could use:
>>> > >
>>> > > [v and type(v) is str for v in config_values.values() if v]
>>> >
>>> > Yeah, they've traditionally just been strings. Added the extra string
>>> check.
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.org/232083003/diff/20001/
>> tools/run-bisect-perf-regression.py#newcode139
>>
>>> > tools/run-bisect-perf-regression.py:139: """Validates that the config
>>> file
>>> > contents is a valid perf config.
>>> > On 2014/04/10 17:58:22, qyearsley wrote:
>>> > > I wasn't aware before that there was such a thing as a "perf config"
>>> (usually
>>> > at
>>> > > run-perf-test.cfg, right?)
>>> > >
>>> > > Maybe add a note saying that a "perf config file" is a config used
>>> when we
>>> > only
>>> > > want to run one perf test once, and we're not actually bisecting;
>>> it's
>>> usually
>>> > > called run-perf-test.cfg by default.
>>> >
>>> > Done.
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.org/232083003/diff/20001/
>> tools/run-bisect-perf-regression.py#newcode440
>>
>>> > tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
>>> > opts.path_to_config
>>> > On 2014/04/10 17:58:22, qyearsley wrote:
>>> > > At this point, we're not 100% sure whether they've supplied a "bisect
>>> config"
>>> > or
>>> > > "perf config".
>>> >
>>> > True, added some changes so that if loading it as a bisect config
>>> fails, it
>>> > falls back to treating it as a perf config.
>>>
>>
>>  LGTM.
>>>
>>
>>  Note: Soon I think we're going to want to add additional optional
>>> parameters
>>>
>> to
>>
>>> the bisect config file, containing information about things like where
>>> to find
>>> the archived builds.
>>>
>>
>>  In light of this, you could also note (if you think it's worth it) that
>>> the
>>> parameter names which are checked are the required parameters; there
>>> could be
>>> additional optional parameters and the validation check would still pass.
>>>
>>
>> Are these new parameters going to be in the config file itself, or passed
>> as
>> parameters to run-bisect-perf-regression.py? Seems like it would be the
>> latter.
>>
>> https://codereview.chromium.org/232083003/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[shatch, 2014-04-10 22:22:47]

On 2014/04/10 18:51:30, prasadv1 wrote:
> correction!
> Let us know if this shouldn't be the case.
> 
> 
> On Thu, Apr 10, 2014 at 11:49 AM, Prasad Vuppalapu
<mailto:prasadv@google.com>wrote:
> 
> > I think these should be a part of config file itself.
> > Since run-bisect-perf-regression.py is called in one of the build step on
> > bisect bot, we might not have control on whether to pass parameters to
> > download archive.
> > Myself and Quinten discussed and felt that modifying config data on demand
> > from the dashboard should give more control whether to download build or
> > not.
> >
> > Let us know if this should be the case.
> >
> >
> > On Thu, Apr 10, 2014 at 11:36 AM, <mailto:simonhatch@chromium.org> wrote:
> >
> >> On 2014/04/10 18:30:24, qyearsley wrote:
> >>
> >>> On 2014/04/10 18:21:11, shatch wrote:
> >>> > New snapshot uploaded, ptal.
> >>> >
> >>> >
> >>>
> >>
> >> https://codereview.chromium.org/232083003/diff/20001/
> >> tools/run-bisect-perf-regression.py
> >>
> >>> > File tools/run-bisect-perf-regression.py (right):
> >>> >
> >>> >
> >>>
> >>
> >> https://codereview.chromium.org/232083003/diff/20001/
> >> tools/run-bisect-perf-regression.py#newcode119
> >>
> >>> > tools/run-bisect-perf-regression.py:119: """Validates that the config
> >>> file
> >>> > contents is a valid perf config, and all
> >>> > On 2014/04/10 17:58:22, qyearsley wrote:
> >>> > > Nit: This wording is a little bit redundant, since "to validate"
> >>> means "to
> >>> > check
> >>> > > whether sth is valid".
> >>> > >
> >>> > > Maybe something like: "Validates the config file contents, checking
> >>>
> >> whether
> >>
> >>> > all
> >>> > > values are non-empty."
> >>> >
> >>> > Done.
> >>> >
> >>> >
> >>>
> >>
> >> https://codereview.chromium.org/232083003/diff/20001/
> >> tools/run-bisect-perf-regression.py#newcode132
> >>
> >>> > tools/run-bisect-perf-regression.py:132: config_has_values = [v for v
> >>> in
> >>> > config_contents.values() if v]
> >>> > On 2014/04/10 17:58:22, qyearsley wrote:
> >>> > > What if the value used for truncate_percent was 0?
> >>> > >
> >>> > > It looks like we're expecting that all of the values in the config
> >>> dict
> >>>
> >> must
> >>
> >>> > be
> >>> > > non-empty strings.
> >>> > >
> >>> > > If you think that later someone might want to directly put integers
> >>> in the
> >>> > > config file, maybe here you could check
> >>> > > [v != '' and v is not None for v in config_values.values() if v]
> >>> > >
> >>> > > OR, if we want to assert that the values in the config must be
> >>> strings,
> >>> > perhaps
> >>> > > you could use:
> >>> > >
> >>> > > [v and type(v) is str for v in config_values.values() if v]
> >>> >
> >>> > Yeah, they've traditionally just been strings. Added the extra string
> >>> check.
> >>> >
> >>> >
> >>>
> >>
> >> https://codereview.chromium.org/232083003/diff/20001/
> >> tools/run-bisect-perf-regression.py#newcode139
> >>
> >>> > tools/run-bisect-perf-regression.py:139: """Validates that the config
> >>> file
> >>> > contents is a valid perf config.
> >>> > On 2014/04/10 17:58:22, qyearsley wrote:
> >>> > > I wasn't aware before that there was such a thing as a "perf config"
> >>> (usually
> >>> > at
> >>> > > run-perf-test.cfg, right?)
> >>> > >
> >>> > > Maybe add a note saying that a "perf config file" is a config used
> >>> when we
> >>> > only
> >>> > > want to run one perf test once, and we're not actually bisecting;
> >>> it's
> >>> usually
> >>> > > called run-perf-test.cfg by default.
> >>> >
> >>> > Done.
> >>> >
> >>> >
> >>>
> >>
> >> https://codereview.chromium.org/232083003/diff/20001/
> >> tools/run-bisect-perf-regression.py#newcode440
> >>
> >>> > tools/run-bisect-perf-regression.py:440: path_to_bisect_cfg =
> >>> > opts.path_to_config
> >>> > On 2014/04/10 17:58:22, qyearsley wrote:
> >>> > > At this point, we're not 100% sure whether they've supplied a "bisect
> >>> config"
> >>> > or
> >>> > > "perf config".
> >>> >
> >>> > True, added some changes so that if loading it as a bisect config
> >>> fails, it
> >>> > falls back to treating it as a perf config.
> >>>
> >>
> >>  LGTM.
> >>>
> >>
> >>  Note: Soon I think we're going to want to add additional optional
> >>> parameters
> >>>
> >> to
> >>
> >>> the bisect config file, containing information about things like where
> >>> to find
> >>> the archived builds.
> >>>
> >>
> >>  In light of this, you could also note (if you think it's worth it) that
> >>> the
> >>> parameter names which are checked are the required parameters; there
> >>> could be
> >>> additional optional parameters and the validation check would still pass.
> >>>
> >>
> >> Are these new parameters going to be in the config file itself, or passed
> >> as
> >> parameters to run-bisect-perf-regression.py? Seems like it would be the
> >> latter.
> >>
> >> https://codereview.chromium.org/232083003/
> >>
> >
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Ah! I wasn't aware this was how you were going. Updated with some more comments.

[shatch, 2014-04-10 22:22:50]

The CQ bit was checked by simonhatch@chromium.org

[commit-bot: I haz the power, 2014-04-10 22:23:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/232083003/60001

[commit-bot: I haz the power, 2014-04-10 23:59:22]

Change committed as 263136