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/04/28 02:59:24 UTC
[GitHub] [beam] pabloem opened a new pull request #11547: All DirectRunners call setup/teardown on DoFns
pabloem opened a new pull request #11547:
URL: https://github.com/apache/beam/pull/11547
**Please** add a meaningful description for your change here
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](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
[GitHub] [beam] pabloem commented on pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11547:
URL: https://github.com/apache/beam/pull/11547#issuecomment-620961633
Ah indeed it should fix that. I'll mark it as fixed.
On Tue, Apr 28, 2020, 7:33 PM Ahmet Altay <no...@github.com> wrote:
> Do you know if this will fix
> https://issues.apache.org/jira/browse/BEAM-7885 ?
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/beam/pull/11547#issuecomment-620959237>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJ5Z3GESQIPKYSS2DGDBNTRO6GY5ANCNFSM4MSOERIA>
> .
>
----------------------------------------------------------------
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
[GitHub] [beam] pabloem commented on pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11547:
URL: https://github.com/apache/beam/pull/11547#issuecomment-620808273
Run Python PreCommit
----------------------------------------------------------------
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
[GitHub] [beam] aaltay commented on pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #11547:
URL: https://github.com/apache/beam/pull/11547#issuecomment-620959237
Do you know if this will fix https://issues.apache.org/jira/browse/BEAM-7885 ?
----------------------------------------------------------------
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
[GitHub] [beam] pabloem commented on pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11547:
URL: https://github.com/apache/beam/pull/11547#issuecomment-620740382
Run Python PreCommit
----------------------------------------------------------------
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
[GitHub] [beam] pabloem commented on a change in pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
pabloem commented on a change in pull request #11547:
URL: https://github.com/apache/beam/pull/11547#discussion_r416784982
##########
File path: sdks/python/apache_beam/runners/direct/transform_evaluator.py
##########
@@ -874,6 +875,7 @@ def process_element(self, element):
def finish_bundle(self):
self.runner.finish()
+ self.runner.teardown()
Review comment:
I need to check this, but in Python, generally, we do not reuse Dofns, so they're recreated for each bundle. Let me confirm..
----------------------------------------------------------------
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
[GitHub] [beam] kennknowles commented on a change in pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
kennknowles commented on a change in pull request #11547:
URL: https://github.com/apache/beam/pull/11547#discussion_r416310620
##########
File path: sdks/python/apache_beam/runners/direct/transform_evaluator.py
##########
@@ -874,6 +875,7 @@ def process_element(self, element):
def finish_bundle(self):
self.runner.finish()
+ self.runner.teardown()
Review comment:
Does the runner re-instantiate the DoFn for each bundle? That is the contract of teardown. The DoFn instance can never be touched again. (I'm mostly trying to learn more about the Python SDK and runners)
----------------------------------------------------------------
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
[GitHub] [beam] pabloem commented on pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
pabloem commented on pull request #11547:
URL: https://github.com/apache/beam/pull/11547#issuecomment-620744960
Run Python PreCommit
----------------------------------------------------------------
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
[GitHub] [beam] aaltay commented on a change in pull request #11547: All Python DirectRunners call setup/teardown on DoFns
Posted by GitBox <gi...@apache.org>.
aaltay commented on a change in pull request #11547:
URL: https://github.com/apache/beam/pull/11547#discussion_r416768530
##########
File path: sdks/python/apache_beam/runners/direct/transform_evaluator.py
##########
@@ -874,6 +875,7 @@ def process_element(self, element):
def finish_bundle(self):
self.runner.finish()
+ self.runner.teardown()
Review comment:
Yes, it does re-instantiate for each bundle. I remember we had a jira somewhere to re-use dofn instances for more than one bundle, we never implemented that.
----------------------------------------------------------------
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