[discuss] merge module flink-yarn and flink-yarn-test

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[discuss] merge module flink-yarn and flink-yarn-test

shijinkui
Hi, All

There too much module in the root. There are no necessary to separate the test code from sub-module.

I never see such design: two modules, one is main code, the other is test code.

Is there some special reason?

From Jinkui Shi
Reply | Threaded
Open this post in threaded view
|

Re: [discuss] merge module flink-yarn and flink-yarn-test

Stephan Ewen
I would like Robert to comment on this.

I think there was a reason to have different modules, which had again
something to do with the Maven Shade Plugin
Dependencies and shading really seem the trickiest thing in bigger
Java/Scala projects ;-)

On Wed, Sep 21, 2016 at 11:04 AM, shijinkui <[hidden email]> wrote:

> Hi, All
>
> There too much module in the root. There are no necessary to separate the
> test code from sub-module.
>
> I never see such design: two modules, one is main code, the other is test
> code.
>
> Is there some special reason?
>
> From Jinkui Shi
>
Reply | Threaded
Open this post in threaded view
|

答复: [discuss] merge module flink-yarn and flink-yarn-test

shijinkui
Hi, Stephan

Thanks for your reply.

In my mind, Maven-shade-plugin and sbt-assembly both default exclude test code for the fat jar.

In fact, unit tests are use to test the main code, ensure our code logic fit our expect . This is general convention. I think. Flink has be a top apache project. We shouldn't be special. We're programmer, should be professional.

Even more, there are `flink-tes-utils-parent` and `flink-tests` module, what's the relation between them.

I have to ask why they are exist? Where is the start of such confusion modules?

I think we shouldn't do nothing for this. Code and design should be comfortable.

Thanks

From Jinkui Shi

-----邮件原件-----
发件人: Stephan Ewen [mailto:[hidden email]]
发送时间: 2016年9月21日 22:19
收件人: [hidden email]
主题: Re: [discuss] merge module flink-yarn and flink-yarn-test

I would like Robert to comment on this.

I think there was a reason to have different modules, which had again something to do with the Maven Shade Plugin Dependencies and shading really seem the trickiest thing in bigger Java/Scala projects ;-)

On Wed, Sep 21, 2016 at 11:04 AM, shijinkui <[hidden email]> wrote:

> Hi, All
>
> There too much module in the root. There are no necessary to separate
> the test code from sub-module.
>
> I never see such design: two modules, one is main code, the other is
> test code.
>
> Is there some special reason?
>
> From Jinkui Shi
>
Reply | Threaded
Open this post in threaded view
|

Re: 答复: [discuss] merge module flink-yarn and flink-yarn-test

Stephan Ewen
"flink-test-utils" contains, as the name says, utils for testing. Intended
to be used by users in writing their own tests.
"flink-tests" contains cross module tests, no user should ever need to have
a dependency on that.

They are different because users explicitly asked for test utils to be
factored into a separate project.

As an honest reply here: Setting up a project as huge as Flink need to take
many things into account

  - Multiple languages (Java / Scala), with limitations of IDEs in mind
  - Dependency conflicts and much shading magic
  - Dependency matrices (multiple hadoop and scala versions)
  - Supporting earlier Java versions
  - clean scope differentiation, so users can reuse utils and testing code


That simply requires some extra modules once in a while. Flink people have
worked hard on coming up with a structure that serves the need of the
production users and automated build/testing systems. These production user
requests are most important to us, and sometimes, we need to take cuts in
"beauty of directory structure" to help them.

Constantly accusing the community of creating bad structures before even
trying to understand the reasoning behind that does not come across as very
friendly. Constantly accusing the community of sloppy work just because
your laptop settings are incompatible with the default configuration
likewise.

I hope you understand that.


On Thu, Sep 22, 2016 at 2:58 AM, shijinkui <[hidden email]> wrote:

> Hi, Stephan
>
> Thanks for your reply.
>
> In my mind, Maven-shade-plugin and sbt-assembly both default exclude test
> code for the fat jar.
>
> In fact, unit tests are use to test the main code, ensure our code logic
> fit our expect . This is general convention. I think. Flink has be a top
> apache project. We shouldn't be special. We're programmer, should be
> professional.
>
> Even more, there are `flink-tes-utils-parent` and `flink-tests` module,
> what's the relation between them.
>
> I have to ask why they are exist? Where is the start of such confusion
> modules?
>
> I think we shouldn't do nothing for this. Code and design should be
> comfortable.
>
> Thanks
>
> From Jinkui Shi
>
> -----邮件原件-----
> 发件人: Stephan Ewen [mailto:[hidden email]]
> 发送时间: 2016年9月21日 22:19
> 收件人: [hidden email]
> 主题: Re: [discuss] merge module flink-yarn and flink-yarn-test
>
> I would like Robert to comment on this.
>
> I think there was a reason to have different modules, which had again
> something to do with the Maven Shade Plugin Dependencies and shading really
> seem the trickiest thing in bigger Java/Scala projects ;-)
>
> On Wed, Sep 21, 2016 at 11:04 AM, shijinkui <[hidden email]> wrote:
>
> > Hi, All
> >
> > There too much module in the root. There are no necessary to separate
> > the test code from sub-module.
> >
> > I never see such design: two modules, one is main code, the other is
> > test code.
> >
> > Is there some special reason?
> >
> > From Jinkui Shi
> >
>
mxm
Reply | Threaded
Open this post in threaded view
|

Re: 答复: [discuss] merge module flink-yarn and flink-yarn-test

mxm
Hello Jinkui Shi,

Due to the nature of most of the Yarn tests, we need them to be in a
separate module. More concretely, these tests have a dependency on
'flink-dist' because they need to deploy the Flink fat jar to the Yarn
tests cluster. The fat jar also contains the 'flink-yarn' code. Thus,
'flink-yarn' needs to be a separate module and built before
'flink-yarn-tests'.

That being said, some of the tests don't need the fat jar, so we could
move some of the tests to 'flink-yarn'. However, that is mostly a
cosmetic change and not important for the testing coverage.

Best,
Max

On Thu, Sep 22, 2016 at 12:26 PM, Stephan Ewen <[hidden email]> wrote:

>
> "flink-test-utils" contains, as the name says, utils for testing. Intended
> to be used by users in writing their own tests.
> "flink-tests" contains cross module tests, no user should ever need to have
> a dependency on that.
>
> They are different because users explicitly asked for test utils to be
> factored into a separate project.
>
> As an honest reply here: Setting up a project as huge as Flink need to take
> many things into account
>
>   - Multiple languages (Java / Scala), with limitations of IDEs in mind
>   - Dependency conflicts and much shading magic
>   - Dependency matrices (multiple hadoop and scala versions)
>   - Supporting earlier Java versions
>   - clean scope differentiation, so users can reuse utils and testing code
>
>
> That simply requires some extra modules once in a while. Flink people have
> worked hard on coming up with a structure that serves the need of the
> production users and automated build/testing systems. These production user
> requests are most important to us, and sometimes, we need to take cuts in
> "beauty of directory structure" to help them.
>
> Constantly accusing the community of creating bad structures before even
> trying to understand the reasoning behind that does not come across as very
> friendly. Constantly accusing the community of sloppy work just because
> your laptop settings are incompatible with the default configuration
> likewise.
>
> I hope you understand that.
>
>
> On Thu, Sep 22, 2016 at 2:58 AM, shijinkui <[hidden email]> wrote:
>
> > Hi, Stephan
> >
> > Thanks for your reply.
> >
> > In my mind, Maven-shade-plugin and sbt-assembly both default exclude test
> > code for the fat jar.
> >
> > In fact, unit tests are use to test the main code, ensure our code logic
> > fit our expect . This is general convention. I think. Flink has be a top
> > apache project. We shouldn't be special. We're programmer, should be
> > professional.
> >
> > Even more, there are `flink-tes-utils-parent` and `flink-tests` module,
> > what's the relation between them.
> >
> > I have to ask why they are exist? Where is the start of such confusion
> > modules?
> >
> > I think we shouldn't do nothing for this. Code and design should be
> > comfortable.
> >
> > Thanks
> >
> > From Jinkui Shi
> >
> > -----邮件原件-----
> > 发件人: Stephan Ewen [mailto:[hidden email]]
> > 发送时间: 2016年9月21日 22:19
> > 收件人: [hidden email]
> > 主题: Re: [discuss] merge module flink-yarn and flink-yarn-test
> >
> > I would like Robert to comment on this.
> >
> > I think there was a reason to have different modules, which had again
> > something to do with the Maven Shade Plugin Dependencies and shading really
> > seem the trickiest thing in bigger Java/Scala projects ;-)
> >
> > On Wed, Sep 21, 2016 at 11:04 AM, shijinkui <[hidden email]> wrote:
> >
> > > Hi, All
> > >
> > > There too much module in the root. There are no necessary to separate
> > > the test code from sub-module.
> > >
> > > I never see such design: two modules, one is main code, the other is
> > > test code.
> > >
> > > Is there some special reason?
> > >
> > > From Jinkui Shi
> > >
> >
Reply | Threaded
Open this post in threaded view
|

答复: 答复: [discuss] merge module flink-yarn and flink-yarn-test

shijinkui
Hi,Maximilian Michels

Thank for your reply.

In order to test submit Flink job to yarn cluster, that is testing client and session cli, split unit test from flink-yarn.
First of all, such yarn cluster unit test should be rename 'flink-yarn-tests' as 'flink-yarn-cluster-test', and move to 'flink-tests' folder as its sub-module.

I think unit test can be divide into module's unit test which can be execute when module is building and unit test module which should be executed independently.
The top module in the root fold shouldn't be a unit test module, not only for good-looking, for its duty.

-----邮件原件-----
发件人: Maximilian Michels [mailto:[hidden email]]
发送时间: 2016年9月26日 16:33
收件人: [hidden email]
主题: Re: 答复: [discuss] merge module flink-yarn and flink-yarn-test

Hello Jinkui Shi,

Due to the nature of most of the Yarn tests, we need them to be in a separate module. More concretely, these tests have a dependency on 'flink-dist' because they need to deploy the Flink fat jar to the Yarn tests cluster. The fat jar also contains the 'flink-yarn' code. Thus, 'flink-yarn' needs to be a separate module and built before 'flink-yarn-tests'.

That being said, some of the tests don't need the fat jar, so we could move some of the tests to 'flink-yarn'. However, that is mostly a cosmetic change and not important for the testing coverage.

Best,
Max

On Thu, Sep 22, 2016 at 12:26 PM, Stephan Ewen <[hidden email]> wrote:

>
> "flink-test-utils" contains, as the name says, utils for testing.
> Intended to be used by users in writing their own tests.
> "flink-tests" contains cross module tests, no user should ever need to
> have a dependency on that.
>
> They are different because users explicitly asked for test utils to be
> factored into a separate project.
>
> As an honest reply here: Setting up a project as huge as Flink need to
> take many things into account
>
>   - Multiple languages (Java / Scala), with limitations of IDEs in mind
>   - Dependency conflicts and much shading magic
>   - Dependency matrices (multiple hadoop and scala versions)
>   - Supporting earlier Java versions
>   - clean scope differentiation, so users can reuse utils and testing
> code
>
>
> That simply requires some extra modules once in a while. Flink people
> have worked hard on coming up with a structure that serves the need of
> the production users and automated build/testing systems. These
> production user requests are most important to us, and sometimes, we
> need to take cuts in "beauty of directory structure" to help them.
>
> Constantly accusing the community of creating bad structures before
> even trying to understand the reasoning behind that does not come
> across as very friendly. Constantly accusing the community of sloppy
> work just because your laptop settings are incompatible with the
> default configuration likewise.
>
> I hope you understand that.
>
>
> On Thu, Sep 22, 2016 at 2:58 AM, shijinkui <[hidden email]> wrote:
>
> > Hi, Stephan
> >
> > Thanks for your reply.
> >
> > In my mind, Maven-shade-plugin and sbt-assembly both default exclude
> > test code for the fat jar.
> >
> > In fact, unit tests are use to test the main code, ensure our code
> > logic fit our expect . This is general convention. I think. Flink
> > has be a top apache project. We shouldn't be special. We're
> > programmer, should be professional.
> >
> > Even more, there are `flink-tes-utils-parent` and `flink-tests`
> > module, what's the relation between them.
> >
> > I have to ask why they are exist? Where is the start of such
> > confusion modules?
> >
> > I think we shouldn't do nothing for this. Code and design should be
> > comfortable.
> >
> > Thanks
> >
> > From Jinkui Shi
> >
> > -----邮件原件-----
> > 发件人: Stephan Ewen [mailto:[hidden email]]
> > 发送时间: 2016年9月21日 22:19
> > 收件人: [hidden email]
> > 主题: Re: [discuss] merge module flink-yarn and flink-yarn-test
> >
> > I would like Robert to comment on this.
> >
> > I think there was a reason to have different modules, which had
> > again something to do with the Maven Shade Plugin Dependencies and
> > shading really seem the trickiest thing in bigger Java/Scala
> > projects ;-)
> >
> > On Wed, Sep 21, 2016 at 11:04 AM, shijinkui <[hidden email]> wrote:
> >
> > > Hi, All
> > >
> > > There too much module in the root. There are no necessary to
> > > separate the test code from sub-module.
> > >
> > > I never see such design: two modules, one is main code, the other
> > > is test code.
> > >
> > > Is there some special reason?
> > >
> > > From Jinkui Shi
> > >
> >