Skip to content

[FLINK-3852] update the quickstart module to include a streaming skeleton#1982

Closed
mred-cmd wants to merge 2 commits intoapache:masterfrom
mred-cmd:FLINK-3852-quickstart-streaming
Closed

[FLINK-3852] update the quickstart module to include a streaming skeleton#1982
mred-cmd wants to merge 2 commits intoapache:masterfrom
mred-cmd:FLINK-3852-quickstart-streaming

Conversation

@mred-cmd
Copy link
Copy Markdown
Contributor

  • Added skeleton StreamingJob
  • Moved Job to BatchJob
  • Commented out transformers for the mainClass setting with a guide to uncomment
  • Updated java and scala docs with a guide on how to run via the cli
  • Updated the site docs to include all four sample classes
  • Tidied up SocketTextStreamWordCount

…ented out transformers for the mainClass setting, tidied up SocketTextStreamWordCount and updated docs
* You will find the jar in
* target/flink-quickstart-0.1-SNAPSHOT-Sample.jar
*
* target/flink-quickstart-0.1-SNAPSHOT.jar
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we update the version in this line too?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I misunderstood. The naming of the jar here depends on the user.

@tzulitai
Copy link
Copy Markdown
Contributor

tzulitai commented May 11, 2016

Made comments on some rather trivial javadoc / comment related stuff, overall the PR looks good!
I like how you've also made effort to tidy up some details in the original code too :)

I'm not a committer, so I would suggest to wait for a committer/maintainer to take a look at the changes too before you proceed with any more modifications.
Otherwise, +1 good to merge from me, after the comments are addressed.

@mred-cmd mred-cmd force-pushed the FLINK-3852-quickstart-streaming branch 5 times, most recently from 3b7e644 to 6cb9a09 Compare May 14, 2016 16:27
… fixed DataSet to DataStream for streaming jobs and fixed indentation
@mred-cmd
Copy link
Copy Markdown
Contributor Author

Thanks for the review @tzulitai I've pushed fixes for all your comments 👍

@rmetzger
Copy link
Copy Markdown
Contributor

I like the pull request! (You updated the docs, the PR describes the changes).

+1 to merge

@StephanEwen
Copy link
Copy Markdown
Contributor

Merging this...

@fhueske
Copy link
Copy Markdown
Contributor

fhueske commented May 17, 2016

Thanks for reworking the quickstart jobs @MarkReddy. Change look good to me.
I am not sure about the naming of the jobs. Should we use DataSetJob instead of BatchJob and DataStreamJob instead of StreamingJob?

@StephanEwen
Copy link
Copy Markdown
Contributor

Race condition ;-)
@fhueske I have locally merged this already. To me the names are good, I would keep them unless you feel strongly about the names.

@fhueske
Copy link
Copy Markdown
Contributor

fhueske commented May 17, 2016

No, names are OK.
Just thought DataSetJob / DataStreamJob would be more consistent with API and documentation.
We can update the names later.

@asfgit asfgit closed this in 3080ea4 May 17, 2016
mbode pushed a commit to mbode/flink that referenced this pull request May 27, 2016
  - move Job to BatchJob
  - comment out transformers for the mainClass setting
  - tidy up SocketTextStreamWordCount
  - update docs

This closes apache#1982
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants