Fixed tools/py_event_log_analyzer/pb_parse.py

Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
NOTRY=True

Review-Url: https://codereview.webrtc.org/2727913004
Cr-Commit-Position: refs/heads/master@{#17776}
Committed: https://chromium.googlesource.com/external/webrtc/+/c2a18c2aae3e199e9ca1e19fe0a8999f6d207f0e

[hillma, 2017-03-03 03:47:58]

The CQ bit was checked by hillma@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 03:48:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[hillma, 2017-03-03 03:48:47]

Description was changed from

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=7289
==========

to

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
==========

[commit-bot: I haz the power, 2017-03-03 04:11:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 04:11:50]

Dry run: This issue passed the CQ dry run.

[kjellander_webrtc, 2017-03-06 14:20:50]

kjellander@webrtc.org changed reviewers:
+ aleloi@webrtc.org, kjellander@webrtc.org

[kjellander_webrtc, 2017-03-06 14:20:51]

Adding aleloi@webrtc.org as reviewer.
Alex: could you please implement a test that will catch errors like this one? I
see the existing one is skipping the entire run if numpy is not found.

[aleloi2, 2017-03-07 15:24:32]

aleloi@google.com changed reviewers:
+ aleloi@google.com

[aleloi2, 2017-03-07 15:24:32]

lgtm. Sorry for the delay. Blaiming travel. I've started looking into a test.

[aleloi, 2017-03-28 16:42:01]

Adding a test to catch this is non-trivial. This script runs under
python-3, but our presubmit is py-2. It can only include the protobuf
files under py-3, because of these reasons:

1. The proto_library template and protoc_wrapper.py put the output
   code in nested directories without '__init__.py' files
2. An old system version of google.protobuf always gets
   included before any other one.

This is of course unfortunate, but I couldn't come up with a better
solution when writing it.

Is there some simple way to run tests under python 3? Is it OK to just
subprocess.call() stuff from inside PRESUBMIT.py?

[kjellander_webrtc, 2017-03-28 18:36:11]

On 2017/03/28 16:42:01, aleloi wrote:
> Adding a test to catch this is non-trivial. This script runs under
> python-3, but our presubmit is py-2. It can only include the protobuf
> files under py-3, because of these reasons:
> 
> 1. The proto_library template and protoc_wrapper.py put the output
>    code in nested directories without '__init__.py' files
> 2. An old system version of google.protobuf always gets
>    included before any other one.
> 
> This is of course unfortunate, but I couldn't come up with a better
> solution when writing it.
> 
> Is there some simple way to run tests under python 3? Is it OK to just
> subprocess.call() stuff from inside PRESUBMIT.py?

I think it's generally preferred to stick to Python 2.7 code, but I cannot find
any style recommendation or Chromium guidelines for it. 
If you rely on Python 3 functionality you should generally use from __future__
import ...
 
Otherwise I guess you could also just check the Python version in the script,
like sys.version_info[:2] > (3,0) etc.

[hlundin-webrtc, 2017-03-29 08:45:31]

On 2017/03/28 18:36:11, kjellander_webrtc wrote:
> On 2017/03/28 16:42:01, aleloi wrote:
> > Adding a test to catch this is non-trivial. This script runs under
> > python-3, but our presubmit is py-2. It can only include the protobuf
> > files under py-3, because of these reasons:
> > 
> > 1. The proto_library template and protoc_wrapper.py put the output
> >    code in nested directories without '__init__.py' files
> > 2. An old system version of google.protobuf always gets
> >    included before any other one.
> > 
> > This is of course unfortunate, but I couldn't come up with a better
> > solution when writing it.
> > 
> > Is there some simple way to run tests under python 3? Is it OK to just
> > subprocess.call() stuff from inside PRESUBMIT.py?
> 
> I think it's generally preferred to stick to Python 2.7 code, but I cannot
find
> any style recommendation or Chromium guidelines for it. 
> If you rely on Python 3 functionality you should generally use from __future__
> import ...
>  
> Otherwise I guess you could also just check the Python version in the script,
> like sys.version_info[:2] > (3,0) etc.

Can we get this landed soon? The tool is currently broken as far as I can tell.

[kjellander_webrtc, 2017-04-19 21:06:36]

Description was changed from

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
==========

to

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
NOTRY=True
==========

[kjellander_webrtc, 2017-04-19 21:06:51]

On 2017/03/29 08:45:31, hlundin-webrtc wrote:
> On 2017/03/28 18:36:11, kjellander_webrtc wrote:
> > On 2017/03/28 16:42:01, aleloi wrote:
> > > Adding a test to catch this is non-trivial. This script runs under
> > > python-3, but our presubmit is py-2. It can only include the protobuf
> > > files under py-3, because of these reasons:
> > > 
> > > 1. The proto_library template and protoc_wrapper.py put the output
> > >    code in nested directories without '__init__.py' files
> > > 2. An old system version of google.protobuf always gets
> > >    included before any other one.
> > > 
> > > This is of course unfortunate, but I couldn't come up with a better
> > > solution when writing it.
> > > 
> > > Is there some simple way to run tests under python 3? Is it OK to just
> > > subprocess.call() stuff from inside PRESUBMIT.py?
> > 
> > I think it's generally preferred to stick to Python 2.7 code, but I cannot
> find
> > any style recommendation or Chromium guidelines for it. 
> > If you rely on Python 3 functionality you should generally use from
__future__
> > import ...
> >  
> > Otherwise I guess you could also just check the Python version in the
script,
> > like sys.version_info[:2] > (3,0) etc.
> 
> Can we get this landed soon? The tool is currently broken as far as I can
tell.

I'll CQ this now.

[kjellander_webrtc, 2017-04-19 21:06:55]

The CQ bit was checked by kjellander@webrtc.org

[commit-bot: I haz the power, 2017-04-19 21:07:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-19 21:12:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-19 21:12:16]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/16265)

[kjellander_webrtc, 2017-04-20 05:55:55]

The CQ bit was checked by kjellander@webrtc.org

[kjellander_webrtc, 2017-04-20 05:55:56]

lgtm

[commit-bot: I haz the power, 2017-04-20 05:56:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-20 05:59:18]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1492667755357510, "parent_rev":
"40fc8766d1dec25552f6ec4a0d1d946624ccefee", "commit_rev":
"c2a18c2aae3e199e9ca1e19fe0a8999f6d207f0e"}

[commit-bot: I haz the power, 2017-04-20 05:59:21]

Description was changed from

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
NOTRY=True
==========

to

==========
Fixed tools/py_event_log_analyzer/pb_parse.py

BUG=webrtc:7289
NOTRY=True

Review-Url: https://codereview.webrtc.org/2727913004
Cr-Commit-Position: refs/heads/master@{#17776}
Committed:
https://chromium.googlesource.com/external/webrtc/+/c2a18c2aae3e199e9ca1e19fe...
==========

[commit-bot: I haz the power, 2017-04-20 05:59:22]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/webrtc/+/c2a18c2aae3e199e9ca1e19fe...