You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@beam.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2019/12/04 00:57:00 UTC

[jira] [Work logged] (BEAM-8739) Consistently use with Pipeline(...) syntax

     [ https://issues.apache.org/jira/browse/BEAM-8739?focusedWorklogId=353152&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-353152 ]

ASF GitHub Bot logged work on BEAM-8739:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Dec/19 00:56
            Start Date: 04/Dec/19 00:56
    Worklog Time Spent: 10m 
      Work Description: robertwb commented on issue #10149: [BEAM-8739] Consistently use with Pipeline(...) syntax
URL: https://github.com/apache/beam/pull/10149#issuecomment-561425881
 
 
   > Before I actually go through all of this, a few general comments:
   > 
   > * I see you did a lot of these conversions automatically. I'd be curious what the scripts looked like.
   
   Yes, I'll attach the script. Nothing fancy. 
   
   > * Ideally we would incorporate this into the linting process, but there's probably enough edge cases to make this infeasible.
   
   Yeah. I'm hoping that mostly people will follow conventions of the surrounding code. 
   
   > * I noticed (as you pointed out on the Jira) that a few tests in here originally didn't call `pipeline.run()` at all (whether through `with` or directly). My biggest concern with this PR is that it might go the other direction and somewhere remove an existing call to `pipeline.run()`. (All other errors I could think of would have resulted in test failures, so you would have caught them already.)
   
   If something is converted to the with syntax, run() will always be called. 
   
   > * If one of the goals here is to encourage everyone to use this syntax, it'd be important to cover the code snippets on the website as well.
   
   Yes. That code is harder to auto (or manually) convert given the amount of mocking and ordering in which things are included/excluded. Taking a pass at the examples and snippets is certainly something that should be done and I'll keep this ticket open until then. 
 
----------------------------------------------------------------
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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 353152)
    Time Spent: 0.5h  (was: 20m)

> Consistently use with Pipeline(...) syntax
> ------------------------------------------
>
>                 Key: BEAM-8739
>                 URL: https://issues.apache.org/jira/browse/BEAM-8739
>             Project: Beam
>          Issue Type: Bug
>          Components: sdk-py-core
>            Reporter: Robert Bradshaw
>            Assignee: Robert Bradshaw
>            Priority: Major
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> I've run into a couple of tests that forgot to do p.run(). In addition, I'm seeing new tests written in this old style. We should consistently use the with syntax where possible for our examples and tests. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)