Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-10791] Provide end-to-end test for Kafka 0.11 connector #7038

Merged
merged 2 commits into from Nov 8, 2018

Conversation

yanghua
Copy link
Contributor

@yanghua yanghua commented Nov 7, 2018

What is the purpose of the change

Provide end-to-end test for Kafka 0.11 connector

Brief change log

  • Provide end-to-end test for Kafka 0.11 connector

Verifying this change

This change is already covered by existing tests, such as (please describe tests).

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
  • The serializers: (yes / no / don't know)
  • The runtime per-record code paths (performance sensitive): (yes / no / don't know)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / no / don't know)
  • The S3 file system connector: (yes / no / don't know)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@yanghua
Copy link
Contributor Author

yanghua commented Nov 7, 2018

@pnowojski @aljoscha can you review this PR? thanks.

@pnowojski
Copy link
Contributor

LGTM, but I've restarted the travis job. I don't understand why RAT check could have failed intermittently.

@yanghua
Copy link
Contributor Author

yanghua commented Nov 7, 2018

Yes, I also encountered this problem. The first time I forgot to add the Apache License to the pom file, but when I added it, it still failed twice and finally succeeded. This PR does not seem to have missing license information.

@pnowojski pnowojski merged commit e3bb113 into apache:master Nov 8, 2018
@pnowojski
Copy link
Contributor

Thanks @yanghua! Merged.

pnowojski pushed a commit to pnowojski/flink that referenced this pull request Nov 8, 2018
…e#7038)

[FLINK-10791][e2e] Provide end-to-end test for Kafka 0.11 connector
Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

I think it is not a good practice to add more examples in order to provide a job for an end-to-end test. It would be better to add this job to the flink-end-to-end-tests module. Now we have the problem that the end-to-end tests require us that the KafkaExamples are in flink-dist which bloats the release binaries unnecessarily up. I think this needs to be reworked.

@yanghua
Copy link
Contributor Author

yanghua commented Nov 19, 2018

@tillrohrmann I agree with your opinion. I have created a new issue (FLINK-10922) to rework specific modules (0.10, 0.11 and 2.0). Will submit PR as soon as possible. cc @pnowojski

@pnowojski
Copy link
Contributor

@tillrohrmann the idea was to to not duplicate the code between examples and end to end tests, which can be (at least in this case) essentially the same.

I'm not sure what's the problem and what would you like to change? To drop Kafka examples from the flink-dist? Don't we want to provide those examples in the distribution? Or we want to provide them only in a form of non compiled code?

@yanghua
Copy link
Contributor Author

yanghua commented Nov 19, 2018

I think @tillrohrmann means that we don't want the e2e test to rely on flink-dist. This way we can run flink-end-to-end-tests directly without packaging the entire project.

But @pnowojski 's considerations are also correct. If we still need to provide an example of the Kafka connector in flink-examples, it will cause some code duplication.

I personally feel that temporary code duplication is acceptable. Over time, e2e's code may change and evolve independently (to test more features), while the Kafka connector only provides a simple usage example.

What do you think?

@tillrohrmann
Copy link
Contributor

I think the Kafka examples don't add much value and should not be shipped with Flink's binary distribution @pnowojski. What we could do is to let flink-end-to-end-tests depend on flink-examples if we want to avoid code duplication.

The problem is that our Flink binaries have grown from 260 MB to almost 500 MB from 1.6 to 1.7. A good part of it is caused by the Kafka examples. It is actually so large that I can no longer commit the binaries via svn to the Apache dev repository.

@yanghua
Copy link
Contributor Author

yanghua commented Nov 19, 2018

I agree that it doesn't make much sense to add Kafka example to Flink binary. Examples released with Flink binary should be as simple and easy to use as possible, and should not include dependencies on other systems.
@tillrohrmann However, I do not recommend that we continue to keep the source code of Kafka example in `flink-examples'(and then let the E2E module depend on it). This module should be mainly used to demonstrate the capabilities (operators) provided by flink, not connectors.
I think we can use the original Kafka example for E2E testing and leave it in the E2E module.

@pnowojski
Copy link
Contributor

@tillrohrmann yes, they do not add much value and yes, they are not worth blowing up the release binaries. However they have some value and especially that we are getting and maintaining them for free (as part of end-to-end tests) it would be nice to have them. Flink generally I think lacks of more code examples.

Maybe the solution is to come up with some structure that will allow us to ship those (and for other connectors/features?) examples as pure code examples?

@yanghua
Copy link
Contributor Author

yanghua commented Nov 20, 2018

@pnowojski How about providing a flink-examples/flink-examples-e2e module to place these end-to-end examples?

@tillrohrmann
Copy link
Contributor

I think it would be nice to have some Flink cookbook with recipes. Using connectors would be one kind of recipes which we could publish there. Ideally, these recipes would be hosted somewhere as real code to avoid that they get out of sync.

tisonkun pushed a commit to tisonkun/flink that referenced this pull request Jan 17, 2019
…e#7038)

[FLINK-10791][e2e] Provide end-to-end test for Kafka 0.11 connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants