You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/03/30 20:26:49 UTC

[GitHub] [beam] tvalentyn opened a new pull request #11265: Fix a Py2/3 incompatibility in profiler.

tvalentyn opened a new pull request #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265
 
 
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) 
   Portable | --- | [![Build Status](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] Hannah-Jiang commented on issue #11265: Fix a Py2/3 incompatibility in profiler.

Posted by GitBox <gi...@apache.org>.
Hannah-Jiang commented on issue #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265#issuecomment-606237227
 
 
   LGTM

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] tvalentyn merged pull request #11265: Fix a Py2/3 incompatibility in profiler.

Posted by GitBox <gi...@apache.org>.
tvalentyn merged pull request #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] tvalentyn commented on issue #11265: Fix a Py2/3 incompatibility in profiler.

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on issue #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265#issuecomment-606230369
 
 
   R: @Hannah-Jiang 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] tvalentyn commented on a change in pull request #11265: Fix a Py2/3 incompatibility in profiler.

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on a change in pull request #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265#discussion_r400511384
 
 

 ##########
 File path: sdks/python/apache_beam/utils/profiler.py
 ##########
 @@ -88,7 +88,11 @@ def __exit__(self, *args):
       self.profile_output = dump_location
 
     if self.log_results:
-      s = io.StringIO()
+      try:
+        import StringIO  # Python 2
 
 Review comment:
   There are several keywords we'll have to look for during this cleanup. "Python 2" should be one of them, that's why I added it. Tracking all possible keywords to search in https://issues.apache.org/jira/browse/BEAM-7372.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [beam] Hannah-Jiang commented on a change in pull request #11265: Fix a Py2/3 incompatibility in profiler.

Posted by GitBox <gi...@apache.org>.
Hannah-Jiang commented on a change in pull request #11265: Fix a Py2/3 incompatibility in profiler.
URL: https://github.com/apache/beam/pull/11265#discussion_r400483969
 
 

 ##########
 File path: sdks/python/apache_beam/utils/profiler.py
 ##########
 @@ -88,7 +88,11 @@ def __exit__(self, *args):
       self.profile_output = dump_location
 
     if self.log_results:
-      s = io.StringIO()
+      try:
+        import StringIO  # Python 2
 
 Review comment:
   NIT: Do we want to add a comment something like `# TODO(BEAM-8371): Python2 should use StringIO` so it's easy to find it when we cleanup Py2 -> Py3? 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services