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.

Sign up for GitHub

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

Jump to bottom

Rebalancing maven module execution order #16497

Merged
merged 1 commit into from
May 6, 2020
Merged

Rebalancing maven module execution order #16497

merged 1 commit into from
May 6, 2020

Conversation

skabashnyuk
Copy link
Contributor

@skabashnyuk skabashnyuk commented Mar 31, 2020 •
edited
Loading

What does this PR do?

Order before rebalancing 
INFO]
[INFO] Che Parent                                                         [pom]
[INFO] Che Core Parent                                                    [pom]
[INFO] Che Core :: Commons :: Parent                                      [pom]
[INFO] Che Core :: Commons :: Annotations                                 [jar]
[INFO] Che Core :: Commons :: Java API extension classes                  [jar]
[INFO] Che Core :: API :: DTO                                             [jar]
[INFO] Che Core :: Commons :: Json helpers                                [jar]
[INFO] Che Core :: Commons :: Utilities for tests                         [jar]
[INFO] Che Core :: Commons :: Dependency Injecting                        [jar]
[INFO] Che Core :: Commons :: Scheduler service                           [jar]
[INFO] Che Core :: API :: DTO Maven Plugin                       [maven-plugin]
[INFO] Che Core :: API :: Core                                            [jar]
[INFO] Che Core :: Commons :: Commons Logback                             [jar]
[INFO] Che Core :: Commons :: XML                                         [jar]
[INFO] Che Core :: Commons :: Commons J2ee                                [jar]
[INFO] Che Core :: Commons :: Tracing and Monitoring wrapper              [jar]
[INFO] Che Core :: Commons :: Mail sender                                 [jar]
[INFO] Che Core :: Commons :: Tracing                                     [jar]
[INFO] Che Core :: API :: TypeScript DTO maven plugin            [maven-plugin]
[INFO] Che Core :: API :: Model                                           [jar]
[INFO] Che Core :: DB                                                     [jar]
[INFO] Che Core :: DB :: Vendor H2                                        [jar]
[INFO] Che Core :: DB :: Vendor MySQL                                     [jar]
[INFO] Che Core :: DB :: Vendor PostgreSQL                                [jar]
[INFO] Che Core :: Tracing :: Core                                        [jar]
[INFO] Che Core :: Tracing :: Web                                         [jar]
[INFO] Che Core :: Metrics :: Core                                        [jar]
[INFO] Che Core :: Tracing :: Metrics                                     [jar]
[INFO] Che Master Parent                                                  [pom]
[INFO] Che Core :: API :: Workspace :: Shared                             [jar]
[INFO] Che Core :: API :: Installer :: Shared                             [jar]
[INFO] Che Core :: API :: Authentication :: Shared                        [jar]
[INFO] Che Core :: API :: Authentication                                  [jar]
[INFO] Che Core :: API :: Authentication Github                           [jar]
[INFO] Che Core :: API :: Authentication OpenShift                        [jar]
[INFO] Che Core :: SQL :: Schema                                          [jar]
[INFO] Che Core :: API :: Account                                         [jar]
[INFO] Che Core :: API :: System Shared                                   [jar]
[INFO] Che Core :: API :: System                                          [jar]
[INFO] Che Core :: API :: SSH :: Shared                                   [jar]
[INFO] Che Core :: API :: User :: Shared                                  [jar]
[INFO] Che Core :: API :: User                                            [jar]
[INFO] Che Core :: API :: SSH                                             [jar]
[INFO] Che Core :: API :: Workspace                                       [jar]
[INFO] Che Multiuser :: Parent                                            [pom]
[INFO] Che Multiuser :: API Parent                                        [pom]
[INFO] Che Multiuser :: Permissions :: Shared                             [jar]
[INFO] Che Multiuser :: Sql Schema                                        [jar]
[INFO] Che Multiuser :: Permissions API                                   [jar]
[INFO] Che Multiuser :: Permissions Parent                                [pom]
[INFO] Che Multiuser :: Workspace Permissions                             [jar]
[INFO] Che Core :: Workspace Activity                                     [jar]
[INFO] Che Core :: API :: Factory :: Shared                               [jar]
[INFO] Che Multiuser :: API :: Authentication Commons                     [jar]
[INFO] Che Multiuser :: API :: Authorization                              [jar]
[INFO] Che Multiuser :: Machine Auth Parent                               [pom]
[INFO] Che Multiuser :: Machine Authentication Shared                     [jar]
[INFO] Che Multiuser :: Machine Authentication                            [jar]
[INFO] Che Infrastructures Parent                                         [pom]
[INFO] Infrastructure :: Kubernetes                                       [jar]
[INFO] Che Core :: API :: Factory                                         [jar]
[INFO] Che Core :: API :: Factory Resolver Github                         [jar]
[INFO] Che Core :: API :: Impl Local                                      [jar]
[INFO] Integration Tests :: Parent                                        [pom]
[INFO] Integration Tests :: Cascade Removal                               [jar]
[INFO] Che Core :: API :: Logger :: Shared                                [jar]
[INFO] Che Core :: API :: Logger                                          [jar]
[INFO] Che Core :: API :: Metrics                                         [jar]
[INFO] Che Multiuser :: API :: Authorization Impl                         [jar]
[INFO] Che Multiuser :: Resource :: Shared                                [jar]
[INFO] Che Multiuser :: Resource                                          [jar]
[INFO] Che Multiuser :: Organization :: Shared                            [jar]
[INFO] Che Multiuser :: Organization                                      [jar]
[INFO] Che Multiuser :: JGroups API Implementation                        [jar]
[INFO] Che Multiuser :: Workspace Activity                                [jar]
[INFO] Che Multiuser :: User Permissions                                  [jar]
[INFO] Che Multiuser :: Devfile Permissions                               [jar]
[INFO] Che Multiuser :: Workspace Activity Permissions                    [jar]
[INFO] Che Multiuser :: Factory Permissions                               [jar]
[INFO] Che Multiuser :: System Permissions                                [jar]
[INFO] Che Multiuser :: Resource :: Permissions                           [jar]
[INFO] Che Multiuser :: Logger Permissions                                [jar]
[INFO] Che Multiuser :: Kubernetes Infrastructure Permissions             [jar]
[INFO] Che Multiuser :: Keycloak Integration Parent                       [pom]
[INFO] Che Multiuser :: Keycloak Shared module                            [jar]
[INFO] Che Multiuser :: Personal account                                  [jar]
[INFO] Che Multiuser :: Keycloak Server                                   [jar]
[INFO] Che Multiuser :: Keycloak Token Provider                           [jar]
[INFO] Che Multiuser :: Integration Tests                                 [pom]
[INFO] Che Multiuser :: Integration Cascade                               [jar]
[INFO] Che Multiuser :: PostgreSQL Tck                                    [jar]
[INFO] Che Multiuser :: MYSQL Tck Tests                                   [jar]
[INFO] Infrastructure :: OpenShift                                        [jar]
[INFO] Infrastructure :: Metrics                                          [jar]
[INFO] Che Assembly :: Parent                                             [pom]
[INFO] Che IDE :: Assemblies Swagger WAR                                  [war]
[INFO] Che IDE :: Compiling WS Master WAR                                 [war]
[INFO] Che IDE :: War Packaging                                           [war]
[INFO] Che Workspace Loader :: War Packaging                              [war]
[INFO] Che IDE :: Dashboard Packaging                                     [war]
[INFO] Che IDE :: Assemblies Che Tomcat8                                  [jar]
[INFO] Che IDE :: Assemblies Tomcat                                       [pom]
[INFO] Che Legacy E2E :: Parent                                           [pom]
[INFO] Che Legacy E2E :: Core                                             [jar]
[INFO] Che Legacy E2E :: Test                                             [jar]
[INFO]
Order after rebalancing 
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Che Parent                                                         [pom]
[INFO] Che Core Parent                                                    [pom]
[INFO] Che Core :: Commons :: Parent                                      [pom]
[INFO] Che Core :: Commons :: Annotations                                 [jar]
[INFO] Che Core :: Commons :: Java API extension classes                  [jar]
[INFO] Che Core :: Commons :: Utilities for tests                         [jar]
[INFO] Che Core :: Commons :: Dependency Injecting                        [jar]
[INFO] Che Core :: Commons :: Json helpers                                [jar]
[INFO] Che Core :: Commons :: XML                                         [jar]
[INFO] Che Core :: Commons :: Scheduler service                           [jar]
[INFO] Che Core :: Commons :: Commons J2ee                                [jar]
[INFO] Che Core :: Commons :: Tracing and Monitoring wrapper              [jar]
[INFO] Che Core :: Commons :: Mail sender                                 [jar]
[INFO] Che Core :: Commons :: Tracing                                     [jar]
[INFO] Che Core :: Commons :: API :: DTO                                  [jar]
[INFO] Che Core :: Commons :: API :: DTO Maven Plugin            [maven-plugin]
[INFO] Che Core :: Commons :: API :: Core                                 [jar]
[INFO] Che Core :: Commons :: API :: TypeScript DTO maven plugin [maven-plugin]
[INFO] Che Core :: Commons :: API :: Model                                [jar]
[INFO] Che Core :: Commons :: DB                                          [jar]
[INFO] Che Core :: Commons :: DB :: Vendor H2                             [jar]
[INFO] Che Core :: Commons :: DB :: Vendor MySQL                          [jar]
[INFO] Che Core :: Commons :: DB :: Vendor PostgreSQL                     [jar]
[INFO] Che Core :: Commons :: Logback                                     [jar]
[INFO] Che Core :: Commons :: Tracing :: Core                             [jar]
[INFO] Che Core :: Commons :: Tracing :: Web                              [jar]
[INFO] Che Core :: Commons :: Metrics :: Core                             [jar]
[INFO] Che Core :: Commons :: Tracing :: Metrics                          [jar]
[INFO] Che Core :: API :: Che Master Parent                               [pom]
[INFO] Che Core :: API :: Workspace :: Shared                             [jar]
[INFO] Che Core :: API :: Installer :: Shared                             [jar]
[INFO] Che Core :: API :: Authentication :: Shared                        [jar]
[INFO] Che Core :: API :: Authentication                                  [jar]
[INFO] Che Core :: API :: Authentication Github                           [jar]
[INFO] Che Core :: API :: Authentication OpenShift                        [jar]
[INFO] Che Core :: SQL :: Schema                                          [jar]
[INFO] Che Core :: API :: Account                                         [jar]
[INFO] Che Core :: API :: System Shared                                   [jar]
[INFO] Che Core :: API :: System                                          [jar]
[INFO] Che Core :: API :: SSH :: Shared                                   [jar]
[INFO] Che Core :: API :: User :: Shared                                  [jar]
[INFO] Che Core :: API :: User                                            [jar]
[INFO] Che Core :: API :: SSH                                             [jar]
[INFO] Che Core :: API :: Workspace                                       [jar]
[INFO] Che Core :: API :: Workspace Activity tracking                     [jar]
[INFO] Che Core :: API :: Factory :: Shared                               [jar]
[INFO] Che Core :: API :: Factory                                         [jar]
[INFO] Che Core :: API :: Factory Resolver Github                         [jar]
[INFO] Che Core :: API :: Impl Local                                      [jar]
[INFO] Che Core :: API :: Logger :: Shared                                [jar]
[INFO] Che Core :: API :: Logger                                          [jar]
[INFO] Che Core :: API :: Metrics                                         [jar]
[INFO] Che Multiuser :: Parent                                            [pom]
[INFO] Che Multiuser :: Sql Schema                                        [jar]
[INFO] Che Multiuser :: API Parent                                        [pom]
[INFO] Che Multiuser :: API :: Authentication Commons                     [jar]
[INFO] Che Multiuser :: Permissions :: Shared                             [jar]
[INFO] Che Multiuser :: Permissions API                                   [jar]
[INFO] Che Multiuser :: API :: Authorization                              [jar]
[INFO] Che Multiuser :: API :: Authorization Impl                         [jar]
[INFO] Che Multiuser :: Resource :: Shared                                [jar]
[INFO] Che Multiuser :: Resource                                          [jar]
[INFO] Che Multiuser :: Organization :: Shared                            [jar]
[INFO] Che Multiuser :: Organization                                      [jar]
[INFO] Che Multiuser :: Workspace Activity                                [jar]
[INFO] Che Multiuser :: Permissions Parent                                [pom]
[INFO] Che Multiuser :: User Permissions                                  [jar]
[INFO] Che Multiuser :: Devfile Permissions                               [jar]
[INFO] Che Multiuser :: Workspace Permissions                             [jar]
[INFO] Che Multiuser :: Workspace Activity Permissions                    [jar]
[INFO] Che Multiuser :: Factory Permissions                               [jar]
[INFO] Che Multiuser :: System Permissions                                [jar]
[INFO] Che Multiuser :: Resource :: Permissions                           [jar]
[INFO] Che Multiuser :: Logger Permissions                                [jar]
[INFO] Che Multiuser :: Keycloak Integration Parent                       [pom]
[INFO] Che Multiuser :: Keycloak Shared module                            [jar]
[INFO] Che Multiuser :: Machine Auth Parent                               [pom]
[INFO] Che Multiuser :: Machine Authentication Shared                     [jar]
[INFO] Che Multiuser :: Personal account                                  [jar]
[INFO] Che Multiuser :: Machine Authentication                            [jar]
[INFO] Che Multiuser :: Keycloak Server                                   [jar]
[INFO] Che Multiuser :: Keycloak Token Provider                           [jar]
[INFO] Che Multiuser :: Integration Tests                                 [pom]
[INFO] Che Multiuser :: Integration Cascade                               [jar]
[INFO] Che Multiuser :: PostgreSQL Tck                                    [jar]
[INFO] Che Multiuser :: MYSQL Tck Tests                                   [jar]
[INFO] Infrastructure :: Infrastructures Parent                           [pom]
[INFO] Infrastructure :: Kubernetes                                       [jar]
[INFO] Infrastructure :: OpenShift                                        [jar]
[INFO] Infrastructure :: Metrics                                          [jar]
[INFO] Infrastructure :: Distributed components                           [jar]
[INFO] Infrastructure :: Kubernetes Permissions                           [jar]
[INFO] Integration Tests :: Parent                                        [pom]
[INFO] Integration Tests :: Cascade Removal                               [jar]
[INFO] Che Assembly :: Parent                                             [pom]
[INFO] Che Assembly :: Swagger war                                        [war]
[INFO] Che Assembly :: WS Master war                                      [war]
[INFO] Che Assembly :: Root war                                           [war]
[INFO] Che Assembly :: Workspace Loader war                               [war]
[INFO] Che Assembly :: Dashboard war                                      [war]
[INFO] Che Assembly :: Assemblies Che Tomcat8                             [jar]
[INFO] Che Assembly :: Assemblies Che Server Tomcat                       [pom]
[INFO] Che Legacy E2E :: Parent                                           [pom]
[INFO] Che Legacy E2E :: Core                                             [jar]
[INFO] Che Legacy E2E :: Test                                             [jar]
[INFO]

PP9DZi8m38NNpIcwHie5OCDDHcvgIaH-ChOp227kFg6Kaj2k-fw_s2zkZon0ahrJUtD6dO0mo46bObgnCVGc4lPtrPTd9efUiTcjKUzZu49ttOTTNBrxuGzLrI1PlLFTgkMMa0tP98qz3erqSHh0NEfp1CQ6gQ4y2bbJpVq4zS22rD-NA2-vN4frt-ckl00k6d07zL1rg-p4PgwwhSvpSZtLXXC12sKZcN3bMvlN6AslBtb0
plantuml

Release Notes

Docs PR

@che-bot che-bot added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 31, 2020
@mshaposhnik
Copy link
Contributor

Why don't put K8S permission stuff right into it's infra as a package? Special module seems excessively for me.

sleshchenko
sleshchenko reviewed Apr 1, 2020
View reviewed changes
Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

So awesome output after rebalancing <3

wsmaster/pom.xml
@@ -42,7 +42,6 @@
<module>che-core-api-ssh-shared</module>
<module>wsmaster-local</module>
<module>che-core-sql-schema</module>
<module>integration-tests</module>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe then it makes more sense to move integration tests folder out from wsmaster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example? What if assembly/integration-tests? Does it make sense to do that in this or we can do that in separate?

Copy link
Member

Choose a reason for hiding this comment

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

Like /tests/integrations-tests?

Does it make sense to do that in this or we can do that in separate?

I don't mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tests/integrations-tests the difficulty that I can see is that tests/integrations-tests probably should run before assembly and test/selenium after.

@skabashnyuk skabashnyuk marked this pull request as ready for review April 1, 2020 11:44
@skabashnyuk skabashnyuk requested review from sparkoo and removed request for l0rd, rhopp, vparfonov and nickboldt April 1, 2020 11:44
@skabashnyuk
Copy link
Contributor Author

Why don't put K8S permission stuff right into it's infra as a package? Special module seems excessively for me.

So we talking about

[INFO] Infrastructure :: Distributed components                           [jar]
[INFO] Infrastructure :: Kubernetes Permissions                           [jar]

Distributed components - related to networking, jgroups. Not sure maybe.
Kubernetes Permissions - related to multiuser/permissions. I would say no. I like it more as independent component that is not related to infrastructure implementation.

@che-bot
Copy link
Contributor

che-bot commented Apr 1, 2020

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Apr 1, 2020

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@dmytro-ndp
Copy link
Contributor

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented Apr 1, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@mshaposhnik
Copy link
Contributor

mshaposhnik commented Apr 2, 2020 •
edited
Loading

Oh. Infrastructure :: Kubernetes Permissions doesn't look like an independent component that is not related to infrastructure implementation. But even if it does, why it in the package where the insfrastructure implementation is ? (i don't like idea to put there anything except the insfastucture implementations at all)

@skabashnyuk
Copy link
Contributor Author

But even if it does, why it in the package where the insfrastructure implementation i

Because it depends on kubernetes-infrastructure module. Where do you want to put it?

@sparkoo
Copy link
Member

sparkoo commented Apr 2, 2020 •
edited
Loading

  • Is there any benefit to have multiuser module as a top-level module? Wouldn't it be more appropriate to have it inside wsmaster ?
  • Is there any benefit to have infrastructure- distributed/metrics/permission in 3 modules? Wouldn't it me simpler to have infrastructure-commons and keep it separated on java packages level?
  • can/want we go further with merging the modules? For example do we really need core/che-core-metrics-core, core/che-core-tracing-core, core/che-core-tracing-metrics, core/che-core-tracing-web ? On the first sight this looks a lot over-engineered, especially when each of these modules have like 5 classes. And I doubt there will ever be a real reason to have them separated on module level. Can they be merged to one module?
  • can we rename modules inside wsmaster to wsmaster? It is very confusing to have che-core* modules in both core and wsmaster

Maybe some of these comments are step ahead and should be discussed later. Straightening the current structure is very good first step.

@skabashnyuk
Copy link
Contributor Author

Is there any benefit to have multiuser module as a top-level module? Wouldn't it be more appropriate to have it inside wsmaster ?

wsmaster is meant to be the place that collects different REST APIs. In case we merge it would make sense to keep distinction flat-base or tree base

wsmaster/
   che-core-api-*
   che-multiuser-*

or

wsmaster/
  /api
  /multiuser

Is there any benefit to have infrastructure- distributed/metrics/permission in 3 modules? Wouldn't it me simpler to have infrastructure-commons and keep it separated on java packages level?

Yes, there is such a benifit. It is based on limiting the scope of the individual modules. In opposite, if you see some module depends on infrastructure-commons that would mean that it also brings jgroups/micrometer/tracing/multiuser staff that might be inappropriate.

can/want we go further with merging the modules? For example do we really need core/che-core-metrics-core, core/che-core-tracing-core, core/che-core-tracing-metrics, core/che-core-tracing-web ? On the first sight this looks a lot over-engineered, especially when each of these modules have like 5 classes. And I doubt there will ever be a real reason to have them separated on module level. Can they be merged to one module?

In theory yes. Motivation same as above. Keep the scope of the module as minimum as possible and do not create uber-jar with multiple responsibilities.

can we rename modules inside wsmaster to wsmaster? It is very confusing to have che-core* modules in both core and wsmaster
Simular to first answer.

wsmaster/
   che-core-*
   che-core-api-*
   che-multiuser-*

or

wsmaster/
  /commons
  /core
  /api
  /multiuser

@skabashnyuk
Rebalancing maven module execution order
e8da129 
Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented May 5, 2020

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

@skabashnyuk
Copy link
Contributor Author

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented May 6, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

@skabashnyuk skabashnyuk merged commit 93ba5ef into eclipse-che:master May 6, 2020
@skabashnyuk skabashnyuk deleted the moduleorder2 branch May 6, 2020 12:54
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label May 6, 2020
Katka92 pushed a commit to Katka92/che that referenced this pull request May 14, 2020
@skabashnyuk @Katka92
Rebalancing maven module execution order (eclipse-che#16497)
a295ff4 
Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sleshchenko sleshchenko sleshchenko left review comments

@mshaposhnik mshaposhnik mshaposhnik approved these changes

@sparkoo sparkoo sparkoo approved these changes

@metlos metlos metlos approved these changes

@amisevsk amisevsk Awaiting requested review from amisevsk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@skabashnyuk @mshaposhnik @che-bot @dmytro-ndp @sparkoo @metlos @sleshchenko