Skip to content

Conversation

@ptomasroos
Copy link
Contributor

In order to be able to run minimal_wordcount on both direct and dataflow I updated the example to reflect that.

@lostluck @herohde

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
--- --- --- ---

@lostluck
Copy link
Contributor

My understanding is that the minimal_wordcount isn't intended to be run on any distributed runner, or serve as a minimal production pipeline.

It's to serve as a toy example to what a beam pipeline looks like, and introduce the model concepts, which can be learned separately. This may not be adequately documented at present. I agree that it doesn't serve as the best tutorial/introduction as of yet. Someone would need to spend the time to improve this.

That said, the other wordcount examples are closer to production style pipelines, so these kinds of updates would be reasonable for them.

@herohde
Copy link
Contributor

herohde commented Sep 14, 2018

Yeah -- minimal_wordcount is written to closely match the Java version and progression of WordCount pipelnes. It's not a good model for pipelines due the the simplifications it makes.

@lostluck
Copy link
Contributor

lostluck commented Sep 17, 2018

@ptomasroos Specifically, this guide:
https://beam.apache.org/get-started/wordcount-example/

Edit: Though it looks like the described changed a bit, without the code changing to match, and there's now Go code in those docs! That would probably be a good resolution to this, for these to match properly.

@ptomasroos ptomasroos changed the title Implemented output flag and correct usage of beamx [BEAM-5378] - Update wordcount_minimal.go to reflect documentation Sep 24, 2018
@ptomasroos ptomasroos changed the title [BEAM-5378] - Update wordcount_minimal.go to reflect documentation [BEAM-5378] - Update minimal_wordcount.go to reflect documentation Sep 24, 2018
@ptomasroos
Copy link
Contributor Author

Ok. I've checked through the Python source-code on what it does, and its aligned with my first changes. I've now updated the documentation of the website as well to reflect this.

Copy link
Contributor

@lostluck lostluck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for your patience Tomas!

I have one question re: if the master repo is now officially where the site documentation needs to be updated. Another recent change had to be applied to the other repo. The Code change LGTM, so we can merge once we hear from Melissa.

{:.language-go}
To view the full code in Go, see
**[wordcount_minimal.go](https://github.com/apache/beam/blob/master/sdks/go/examples/minimal_wordcount/minimal_wordcount.go).**
**[minimal_wordcount.go](https://github.com/apache/beam/blob/master/sdks/go/examples/minimal_wordcount/minimal_wordcount.go).**
Copy link
Contributor

@lostluck lostluck Sep 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I don't think the site uses this copy in master just yet.

@melap Is that still true? Does this change need to go into the old site repo still?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, changes to website markdown still need to be done in beam-site until the migration is complete

@stale
Copy link

stale bot commented Nov 26, 2018

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.

@stale stale bot added the stale label Nov 26, 2018
@stale
Copy link

stale bot commented Dec 3, 2018

This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@stale stale bot closed this Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants