Skip to content

[BEAM-664] Update Java MinimalWordCount instructions#336

Closed
alex-filatov wants to merge 1 commit intoapache:asf-sitefrom
alex-filatov:beam-664-update-docs
Closed

[BEAM-664] Update Java MinimalWordCount instructions#336
alex-filatov wants to merge 1 commit intoapache:asf-sitefrom
alex-filatov:beam-664-update-docs

Conversation

@alex-filatov
Copy link

@alex-filatov alex-filatov commented Oct 19, 2017

MinimalWordCount in Java is intentionally hardcoded to run only on DirectRunner.

@aaltay
Copy link
Member

aaltay commented Oct 20, 2017

Hey @alex-filatov, could you explain this more? Should we instead change the example to not-hardcode to work only one runner.

cc: @kennknowles in case he has more context.

@alex-filatov
Copy link
Author

@aaltay @kennknowles Current instructions are broken: you'll get exception if you try to run Java MinimalWordCount using e.g. Flink runner. Checking history, it appears this was a deliberate change. Commit message: "This makes it easy to immediately run, and removes various
non-portable instructions and others that aren't the easiest for a "Getting Started" scenario." This makes sense to me.

Should we instead change the example to not-hardcode to work only one runner.

Not sure. Beam model is independent of the runners and could be explained separately (using Direct Runner) from the real runners. I would change all examples to use Direct Runner only and provide separate instructions how to specify and configure different runners. IMO this decoupling would make docs simpler.

@kennknowles
Copy link
Member

The context is that just to keep it truly "minimal" as a demonstration of how pithy we could get things with Java 8, we hardcode everything. The idea is that if you want to experiment with it you edit the code. It is just the tiniest example.

@kennknowles
Copy link
Member

That was back in a time where runners needed more special consideration than they do now. As long as the code for MinimalWordCount stays very concise, I am happy to have command lines that run on different runners.

@aaltay
Copy link
Member

aaltay commented Oct 25, 2017

@kennknowles My understanding so far is, {{MinimalWordCount}} needs pipeline options for runner, input, output to be able to run with other runners. We do not want to do that to complicate things. In that case, making this documentation changes makes sense. Is this accurate?

@kennknowles
Copy link
Member

Actually it has harcoded input and hardcoded output. I do think that you could set the runner via pipeline options and it would work.

@aaltay
Copy link
Member

aaltay commented Oct 25, 2017

Does the hardcoded input/output work for all runners as it is?

@kennknowles
Copy link
Member

I'm actually not sure. FWIW this PR is clearly an improvement as the bits it deletes are passing command line args that don't exist. I expect they would actually cause the command to fail.

@aaltay
Copy link
Member

aaltay commented Oct 26, 2017

I agree that this PR is an improvement. I can merge it unless there is an objection.

cc: @melap

@melap
Copy link

melap commented Oct 26, 2017

LGTM

@asfgit asfgit closed this in 0cf69dd Oct 27, 2017
@alex-filatov alex-filatov deleted the beam-664-update-docs branch November 19, 2017 20:03
robertwb pushed a commit to robertwb/incubator-beam that referenced this pull request Jun 5, 2018
robertwb pushed a commit to robertwb/incubator-beam that referenced this pull request Jun 5, 2018
melap pushed a commit to apache/beam that referenced this pull request Jun 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants