[Android] memory_inspector: update PRESUBMIT.py to check prebuilts.

[Android] memory_inspector: update PRESUBMIT.py to check prebuilts.

Add a PRESUBMIT check to verify that all prebuilts are actually
deployed to GCS (i.e. the committer didn't forget to run
upload_to_google_storage.py).

BUG=340294
NOTRY=true

Committed: https://crrev.com/75a9e5a0b0ec4f9397749d6f456e3c2553ecf781
Cr-Commit-Position: refs/heads/master@{#295057}

[Primiano Tucci (use gerrit), 2014-09-10 17:36:58]

Patchset #1 (id:1) has been deleted

[Primiano Tucci (use gerrit), 2014-09-10 17:40:57]

primiano@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Primiano Tucci (use gerrit), 2014-09-10 17:40:57]

Hi Pawel,
Can ask you a review on this CL?
I'm adding a presubmit check to check whether some prebuilts are uploaded on
GCS.
Is it fine doing so in a presubmit check (only pre-upload)?

Thanks!

[Paweł Hajdan Jr., 2014-09-11 09:10:06]

phajdan.jr@chromium.org changed reviewers:
+ maruel@chromium.org

[Paweł Hajdan Jr., 2014-09-11 09:10:06]

I defer to Marc-Antoine.

[M-A Ruel, 2014-09-11 12:33:17]

You modify sys.path, you need to dupe it, not call insert()

[Primiano Tucci (use gerrit), 2014-09-11 13:13:15]

Good catch actually, thanks.

[M-A Ruel, 2014-09-12 13:58:37]

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
File tools/memory_inspector/PRESUBMIT.py (right):

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:36: old_sys_path = sys.path[:]
Actually you want to do the reverse;

old_sys_path = sys.path
try:
  sys.path = [input_api.PresubmitLocalPath()] + sys.path
  ...
finally:
  sys.path = old_sys_path

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:49: url = constants.PREBUILTS_BASE_URL + sha
Is it https?

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:53: urllib2.urlopen(request)
Use a timeout

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:54: except Exception, e:
Exception as e:

[Primiano Tucci (use gerrit), 2014-09-12 15:30:42]

Many thanks M-A for your time!
See replies inline.

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
File tools/memory_inspector/PRESUBMIT.py (right):

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:36: old_sys_path = sys.path[:]
On 2014/09/12 13:58:37, M-A Ruel wrote:
> Actually you want to do the reverse;
> 
> old_sys_path = sys.path
> try:
>   sys.path = [input_api.PresubmitLocalPath()] + sys.path
>   ...
> finally:
>   sys.path = old_sys_path
Uh? I'm fine both ways.
Can I ask just you what would be the difference between creating a copy before,
mangling the original path and reassigning the copy versus assigning the path to
a new mangled one and restoring the original?

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:49: url = constants.PREBUILTS_BASE_URL + sha
On 2014/09/12 13:58:37, M-A Ruel wrote:
> Is it https?

Yes, it is
PREBUILTS_BASE_URL = 'https://storage.googleapis.com/chromium-telemetry/'

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:53: urllib2.urlopen(request)
On 2014/09/12 13:58:37, M-A Ruel wrote:
> Use a timeout

Done.

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:54: except Exception, e:
On 2014/09/12 13:58:37, M-A Ruel wrote:
> Exception as e:

Done.

[Primiano Tucci (use gerrit), 2014-09-16 09:50:37]

PING. Does it look good now?

[M-A Ruel, 2014-09-16 13:00:19]

lgtm

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
File tools/memory_inspector/PRESUBMIT.py (right):

https://codereview.chromium.org/562553003/diff/40001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:36: old_sys_path = sys.path[:]
On 2014/09/12 15:30:41, Primiano Tucci wrote:
> On 2014/09/12 13:58:37, M-A Ruel wrote:
> > Actually you want to do the reverse;
> > 
> > old_sys_path = sys.path
> > try:
> >   sys.path = [input_api.PresubmitLocalPath()] + sys.path
> >   ...
> > finally:
> >   sys.path = old_sys_path
> Uh? I'm fine both ways.
> Can I ask just you what would be the difference between creating a copy
before,
> mangling the original path and reassigning the copy versus assigning the path
to
> a new mangled one and restoring the original?

Well, the original could be something else than a list. Technically, it could be
anything implementing a list-like behavior, this is python. :)

https://codereview.chromium.org/562553003/diff/60001/tools/memory_inspector/P...
File tools/memory_inspector/PRESUBMIT.py (right):

https://codereview.chromium.org/562553003/diff/60001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:33: def
_CheckPrebuiltsAreUploaded(input_api, output_api):
Could you add a short docstring about what it does, the implications and why?

[Primiano Tucci (use gerrit), 2014-09-16 13:48:03]

Thanks

https://codereview.chromium.org/562553003/diff/60001/tools/memory_inspector/P...
File tools/memory_inspector/PRESUBMIT.py (right):

https://codereview.chromium.org/562553003/diff/60001/tools/memory_inspector/P...
tools/memory_inspector/PRESUBMIT.py:33: def
_CheckPrebuiltsAreUploaded(input_api, output_api):
On 2014/09/16 13:00:19, M-A Ruel wrote:
> Could you add a short docstring about what it does, the implications and why?

Done.

[Primiano Tucci (use gerrit), 2014-09-16 13:48:06]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-09-16 13:48:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/562553003/80001

[commit-bot: I haz the power, 2014-09-16 13:50:24]

Committed patchset #4 (id:80001) as e97525385dd5158270ef9ab41637c0ef8805142e

[commit-bot: I haz the power, 2014-09-16 13:53:17]

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