telemetry: Create a safe url generator.

telemetry: Create a safe url generator.

The URLs are used to generate fake Chromium profiles.

BUG=

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

[erikchen, 2015-02-12 01:29:30]

Patchset #1 (id:1) has been deleted

[erikchen, 2015-02-12 01:29:34]

Patchset #1 (id:20001) has been deleted

[erikchen, 2015-02-12 01:29:38]

Patchset #1 (id:40001) has been deleted

[erikchen, 2015-02-12 01:30:55]

erikchen@chromium.org changed reviewers:
+ nednguyen@google.com

[erikchen, 2015-02-12 01:30:56]

nednguyen: Please review.

[nednguyen, 2015-02-12 01:43:46]

nednguyen@google.com changed reviewers:
+ sullivan@chromium.org

[nednguyen, 2015-02-12 01:43:47]

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
File tools/perf/profile_creators/profile_safe_url_generator.py (right):

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
tools/perf/profile_creators/profile_safe_url_generator.py:13: from bs4 import
BeautifulSoup
Does bot machine has bs4?

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
tools/perf/profile_creators/profile_safe_url_list.py:22: url_list = [
Why embed this list in python file? Can you make a .json file that contain this
list instead?

[erikchen, 2015-02-12 01:46:57]

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
File tools/perf/profile_creators/profile_safe_url_generator.py (right):

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
tools/perf/profile_creators/profile_safe_url_generator.py:13: from bs4 import
BeautifulSoup
On 2015/02/12 01:43:46, nednguyen wrote:
> Does bot machine has bs4?

No.

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
tools/perf/profile_creators/profile_safe_url_list.py:22: url_list = [
On 2015/02/12 01:43:46, nednguyen wrote:
> Why embed this list in python file? Can you make a .json file that contain
this
> list instead?

This is the way that existing telemetry code embeds large lists. See 
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_se...

[erikchen, 2015-02-12 03:20:01]

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
tools/perf/profile_creators/profile_safe_url_list.py:22: url_list = [
On 2015/02/12 01:46:57, erikchen wrote:
> On 2015/02/12 01:43:46, nednguyen wrote:
> > Why embed this list in python file? Can you make a .json file that contain
> this
> > list instead?
> 
> This is the way that existing telemetry code embeds large lists. See 
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_se...

Having looked through the existing page_sets, the example I gave is the
exception, not the rule. Updating to JSON now.

[nednguyen, 2015-02-12 03:24:27]

On 2015/02/12 03:20:01, erikchen wrote:
>
https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
> File tools/perf/profile_creators/profile_safe_url_list.py (right):
> 
>
https://codereview.chromium.org/918893002/diff/60001/tools/perf/profile_creat...
> tools/perf/profile_creators/profile_safe_url_list.py:22: url_list = [
> On 2015/02/12 01:46:57, erikchen wrote:
> > On 2015/02/12 01:43:46, nednguyen wrote:
> > > Why embed this list in python file? Can you make a .json file that contain
> > this
> > > list instead?
> > 
> > This is the way that existing telemetry code embeds large lists. See 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/page_se...
> 
> Having looked through the existing page_sets, the example I gave is the
> exception, not the rule. Updating to JSON now.

Yeah, I will fix that one to avoid further bad style copying in the future :-)

[erikchen, 2015-02-12 03:29:44]

Patchset #2 (id:80001) has been deleted

[erikchen, 2015-02-12 03:31:32]

nednguyen: PTAL

[nednguyen, 2015-02-12 04:55:11]

nednguyen@google.com changed reviewers:
+ dtu@chromium.org

[nednguyen, 2015-02-12 04:55:12]

+Dave: should we allow adding beautiful_soup into telemetry's third party
library?

[dtu, 2015-02-12 23:46:06]

On 2015/02/12 04:55:12, nednguyen wrote:
> +Dave: should we allow adding beautiful_soup into telemetry's third party
> library?

We don't have any policy for third_party yet, but probably should. What're the
downsides? Potential API and code size bloat?

I think the usage here is on the simpler side, and you can use the Python
standard library HTMLParser or something for this.

[dtu, 2015-02-12 23:46:12]

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_generator.py (right):

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_generator.py:14: from bs4 import
BeautifulSoup
style guide nit: don't import classes directly

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:7: 
style guide nit: 2 blank spaces between top level blocks

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:22: return
json.loads(json_data)
To ensure prompt file closing (otherwise file is closed at garbage collection):

with open(safe_url_path, 'r') as safe_url_file:
  return json.load(safe_url_file)

[nednguyen, 2015-02-13 00:17:53]

On 2015/02/12 23:46:06, dtu wrote:
> On 2015/02/12 04:55:12, nednguyen wrote:
> > +Dave: should we allow adding beautiful_soup into telemetry's third party
> > library?
> 
> We don't have any policy for third_party yet, but probably should. What're the
> downsides? Potential API and code size bloat?
> 
> I think the usage here is on the simpler side, and you can use the Python
> standard library HTMLParser or something for this.

+1 Please use existing library if you can

[erikchen, 2015-02-13 03:57:31]

Patchset #3 (id:120001) has been deleted

[erikchen, 2015-02-13 03:58:58]

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_generator.py (right):

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_generator.py:14: from bs4 import
BeautifulSoup
On 2015/02/12 23:46:12, dtu wrote:
> style guide nit: don't import classes directly

Done.

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:7: 
On 2015/02/12 23:46:12, dtu wrote:
> style guide nit: 2 blank spaces between top level blocks

Done.

https://codereview.chromium.org/918893002/diff/100001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:22: return
json.loads(json_data)
On 2015/02/12 23:46:12, dtu wrote:
> To ensure prompt file closing (otherwise file is closed at garbage
collection):
> 
> with open(safe_url_path, 'r') as safe_url_file:
>   return json.load(safe_url_file)

Ah, good point.

[erikchen, 2015-02-13 03:59:31]

dtu: PTAL

Using HTMLParser is a much nicer solution than using BS4. Thanks for the
suggestion!

[dtu, 2015-02-13 23:00:00]

lgtm % nit

https://codereview.chromium.org/918893002/diff/140001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/140001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:24: return
json.load(safe_url_file)
nit: 2 space indent

[erikchen, 2015-02-13 23:11:04]

New patchsets have been uploaded after l-g-t-m from dtu@chromium.org

[erikchen, 2015-02-13 23:11:24]

https://codereview.chromium.org/918893002/diff/140001/tools/perf/profile_crea...
File tools/perf/profile_creators/profile_safe_url_list.py (right):

https://codereview.chromium.org/918893002/diff/140001/tools/perf/profile_crea...
tools/perf/profile_creators/profile_safe_url_list.py:24: return
json.load(safe_url_file)
On 2015/02/13 23:00:00, dtu wrote:
> nit: 2 space indent

Done.

[erikchen, 2015-02-13 23:11:31]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-02-13 23:13:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/918893002/160001

[commit-bot: I haz the power, 2015-02-14 00:14:23]

Committed patchset #4 (id:160001)

[commit-bot: I haz the power, 2015-02-14 00:15:16]

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