[Telemetry] Cleanup some unused variables in page_sets.

[Telemetry] Cleanup some unused variables in page_sets.

BUG=

Committed: https://crrev.com/aae535ca8e8087942b4716f92cf833f84a38f1a9
Cr-Commit-Position: refs/heads/master@{#380176}

[mikecase (-- gone --), 2016-03-08 02:18:56]

mikecase@chromium.org changed reviewers:
+ perezju@chromium.org

[mikecase (-- gone --), 2016-03-08 02:18:56]

When reading through some of the page set code it looked as if there were some
unused
variables (mostly archive_data_file which doesn't appear at all in the Page
module and isn't
used in the page_sets). Thought I would clean these up.

[perezju, 2016-03-08 11:09:14]

perezju@chromium.org changed reviewers:
+ aiolos@chromium.org, nednguyen@google.com

[perezju, 2016-03-08 11:09:14]

Hmm, I guess you are right. Did you check that the benchmarks still work? Try
using a device on airplane mode, to ensure this doesn't break WPR.

Also adding Telemety experts to double check.

[nednguyen, 2016-03-08 17:13:18]

lgtm

I think this clean up is correct. This was the results of the automatic tool I
wrote that translate json page set to python.

Can you please update with perf-sherrif before landing this?

[nednguyen, 2016-03-08 17:13:36]

On 2016/03/08 17:13:18, nednguyen wrote:
> lgtm
> 
> I think this clean up is correct. This was the results of the automatic tool I
> wrote that translate json page set to python.
> 
> Can you please update with perf-sherrif before landing this?

Thanks for the awesome cleanup, Mike!

[aiolos (Not reviewing), 2016-03-08 17:43:49]

Not lgtm.

archive_data_file is used in
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
to determine what wpr archive(s) the story set depends on. Deleting it from the
page sets should break those benchmarks (at the very least when the wpr files
aren't already downloaded).

It's passed through here in story_set.py:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...

It shouldn't get deleted until the archive_data file is replaced with a
dependency manager instance.

[nednguyen, 2016-03-08 18:01:44]

On 2016/03/08 17:43:49, aiolos wrote:
> Not lgtm.
> 
> archive_data_file is used in
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> to determine what wpr archive(s) the story set depends on. Deleting it from
the
> page sets should break those benchmarks (at the very least when the wpr files
> aren't already downloaded).
> 
> It's passed through here in story_set.py:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> 
> It shouldn't get deleted until the archive_data file is replaced with a
> dependency manager instance.

Mike is deleting these archive_data file from _pages_, not _story_sets_

[aiolos (Not reviewing), 2016-03-08 18:10:05]

On 2016/03/08 18:01:44, nednguyen wrote:
> On 2016/03/08 17:43:49, aiolos wrote:
> > Not lgtm.
> > 
> > archive_data_file is used in
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> > to determine what wpr archive(s) the story set depends on. Deleting it from
> the
> > page sets should break those benchmarks (at the very least when the wpr
files
> > aren't already downloaded).
> > 
> > It's passed through here in story_set.py:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> > 
> > It shouldn't get deleted until the archive_data file is replaced with a
> > dependency manager instance.
> 
> Mike is deleting these archive_data file from _pages_, not _story_sets_

oh, sorry. I apparently wasn't looking close enough. Why do we even have that
lever?

lgtm.

[nednguyen, 2016-03-08 18:21:46]

On 2016/03/08 18:10:05, aiolos wrote:
> On 2016/03/08 18:01:44, nednguyen wrote:
> > On 2016/03/08 17:43:49, aiolos wrote:
> > > Not lgtm.
> > > 
> > > archive_data_file is used in
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> > > to determine what wpr archive(s) the story set depends on. Deleting it
from
> > the
> > > page sets should break those benchmarks (at the very least when the wpr
> files
> > > aren't already downloaded).
> > > 
> > > It's passed through here in story_set.py:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...
> > > 
> > > It shouldn't get deleted until the archive_data file is replaced with a
> > > dependency manager instance.
> > 
> > Mike is deleting these archive_data file from _pages_, not _story_sets_
> 
> oh, sorry. I apparently wasn't looking close enough. Why do we even have that
> lever?
> 
> lgtm.

Long time ago, telemetry blurs the boundary between page sets & pages inside
page sets. If an attribute is defined on page sets, it's also defined on all
pages. That was for the purpose of logic sharing because people don't want to
copy & paste the same json attributes all over the places. 

When I convert json page sets to python using automatic tool, for safety reason,
I just duplicate all the page set attributes to page classes.

[mikecase (-- gone --), 2016-03-09 18:05:14]

The CQ bit was checked by mikecase@chromium.org

[commit-bot: I haz the power, 2016-03-09 18:05:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1767413002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1767413002/1

[commit-bot: I haz the power, 2016-03-09 19:34:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-09 19:35:14]

Description was changed from

==========
[Telemetry] Cleanup some unused variables in page_sets.

BUG=
==========

to

==========
[Telemetry] Cleanup some unused variables in page_sets.

BUG=

Committed: https://crrev.com/aae535ca8e8087942b4716f92cf833f84a38f1a9
Cr-Commit-Position: refs/heads/master@{#380176}
==========

[commit-bot: I haz the power, 2016-03-09 19:35:15]

Patchset 1 (id:??) landed as
https://crrev.com/aae535ca8e8087942b4716f92cf833f84a38f1a9
Cr-Commit-Position: refs/heads/master@{#380176}