Skip to content

Conversation

tillrohrmann
Copy link
Contributor

This PR effectively removes all unconditioned outputs of the usage message of the examples.

@vasia
Copy link
Contributor

vasia commented Mar 1, 2016

If we remove the default usage printing, I think it would be nice to make sure that all examples implement ProgramDescription. This way, we can get the usage information if we want to. Otherwise, we would get one message per default parameter. e.g. in the Kmeans example, we would get:

Executing K-Means example with default point data set.
Use --points to specify file input.
Executing K-Means example with default centroid data set.
Use --centroids to specify file input.
Printing result to stdout. Use --output to specify output path.

instead of
Usage: KMeans --points <path> --centroids <path> --output <path> --iterations <n>

@tillrohrmann
Copy link
Contributor Author

That is a good point @vasia. Will add the missing ProgramDescriptions.

@tillrohrmann
Copy link
Contributor Author

Added the missing ProgramDescriptions.

@StephanEwen
Copy link
Contributor

I remember a discussion from a while back that we actually wanted to drop ProgramDescription.

It comes from the days when programs had to implement specific interface and it does not really go together very well with the idea of a simple static main-method as the entry point.

@uce
Copy link
Contributor

uce commented Mar 14, 2016

I think Stephan has a point and we should maybe not encourage usage of ProgramDescription in the examples, because we advertise the simple main-method entry point in all the docs.

Other than that, I think it's a good idea to remove the unconditional usage printing.

@tillrohrmann
Copy link
Contributor Author

ProgramDescription has not been marked as deprecated so far. Furthermore it can be used to show information about the program using bin/flink info and is not mandatory.

Admittedly, this feature isn't used by many programs but I think it does not hurt to keep and implement this interface, unless we plan to remove it soon. There was a discussion which also involved ProgramDescription. However, it does not look to me as if it converged to some real action points.

@fhueske
Copy link
Contributor

fhueske commented Mar 22, 2016

I don't think the examples should implement the ProgramDescription interface. This might give the impression that implementing this interface is mandatory while it is a very optional thing.

I think it is not a problem that optional program parameters such as max-iterations won't be printed anymore. I would expect users to look at the code anyways to understand what is happening and there they will see the optional parameters.

@tillrohrmann
Copy link
Contributor Author

Hmm but if it is nowhere implemented then we could directly remove ProgramDescription. I mean why keeping code in the base which is not used?

Furthermore, I don't think that it would harm if people implemented this interface. And if they look in the code, then they will also see that it's highly optional.

@StephanEwen
Copy link
Contributor

From my side, +1 for removing the ProgramDescription interface.

I think it is a mismatch with the main() method based approach, because it needs to instantiate the class, while the general entry point is static. That does not go well together in my opinion. People will write programs with no default constructor. Previously, with the Program interface, such a constructor was also mandatory, it did fit better there.

@tillrohrmann
Copy link
Contributor Author

That is a good point with the instantiation of the instances. I will update the PR accordingly.

@tillrohrmann
Copy link
Contributor Author

Removed the ProgramDescription interface and rebase the PR on the latest master.

@uce
Copy link
Contributor

uce commented Apr 25, 2016

Thanks, Till! I think this is ready to finally be merged. I will go ahead and do it.

@asfgit asfgit closed this in 44c2d2f Apr 25, 2016
kl0u pushed a commit to kl0u/flink that referenced this pull request Apr 29, 2016
StefanRRichter pushed a commit to StefanRRichter/flink that referenced this pull request May 3, 2016
@tillrohrmann tillrohrmann deleted the fixExamples branch August 19, 2016 12:13
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.

6 participants