[NU-1777] Separate classloader for deployment managers #7335

coutoPL · 2024-12-13T16:38:20Z

Describe your changes

deployment managers have a separate class loader (the Application ClassLoader is no longer used to load their classe
deployment manager class loader is a parent for the model class loader
you can configure paths for deployment managers using the managersDirs config setting
removed unused env used at the SBT level (includeFlinkAndScala)
new E2E test (example R-R scenario)

Checklist before merge

Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
Code is cleaned from temporary changes and commented out lines
Parts of the code that are not easy to understand are documented in the code
Changes are covered by automated tests
Showcase in dev-application.conf added to demonstrate the feature
Documentation added or updated
Added entry in Changelog.md describing the change from the perspective of a public distribution user
Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
Verify that PR will be squashed during merge

arkadius · 2024-12-16T21:19:03Z

build.sbt

@@ -438,7 +456,8 @@ def assemblySettings(
    includeScala: Boolean,
    filterProvidedDeps: Boolean = true
 ): List[Def.SettingsDefinition] = {
-  // This work around need to be optional because for designer module it causes excluding of scala lib (because we has there other work around for Idea classpath and provided deps)
+  // This work around need to be optional because for designer module it causes excluding of scala lib
+  // (because we have there other work around for Idea classpath and provided deps)


Is it still relevant?

TBH I'm not sure what exact problem was solved this way and how to check if my change outdated it.

I see that false was passed only for designer assembly. It looks like we have a fat jar for designer. I don't know if someone uses it :) but ... The only provided dependencies of designer where DMs so the default filterProvidedDeps = true should work the same so we can remove this condition

arkadius · 2024-12-16T21:22:32Z

build.sbt

      Def
-        .taskDyn(if (addManagerArtifacts) distribution / managerArtifacts else Def.task[List[(File, String)]](Nil))
+        .taskDyn(
+          if (addManagerArtifacts) distribution / deploymentManagerArtifacts else Def.task[List[(File, String)]](Nil)


Won't we build some artefacts twice? I see that developmentTestsDeployManagerArtifacts contains also production artifacts now. Or sbt deduplicate them?

designer/server/src/main/scala/pl/touk/nussknacker/ui/api/description/NodesApiEndpoints.scala

arkadius · 2024-12-16T21:32:29Z

scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/ModelData.scala

@@ -125,8 +128,6 @@ case class ClassLoaderModelData private (
    override val additionalConfigsFromProvider: Map[DesignerWideComponentId, ComponentAdditionalConfig],
    // This property is for easier testing when for some reason, some jars with ComponentProvider are
    // on the test classpath and CPs collide with other once with the same name.
-    // E.g. we add liteEmbeddedDeploymentManager as a designer provided dependency which also


I would remove these functions now and whole comment block

I wanted to remove it but there is still some problem with ProcessConfigCreator implementations. Nevertheless, the lite Embedded deployment manager is not the problem anymore.

build.sbt

arkadius · 2024-12-16T21:43:50Z

build.sbt

@@ -2036,6 +2057,8 @@ lazy val designer = (project in file("designer/server"))
        "io.circe"                      %% "circe-yaml"                      % circeYamlV           % Test,
        "com.github.scopt"              %% "scopt"                           % "4.1.0"              % Test,
        "org.questdb"                    % "questdb"                         % "7.4.2",
+        "org.apache.kafka"               % "kafka-clients"                   % kafkaV,


What's going on here?

TBH this dependency should be added at the component definition level (in the module when the kafka-related component/s is/are added). For some reason, there is no such dependency in the lite kafka component module (or one of the lite-related kafka modules). It's hard for me to unravel the relationships between these modules, so I decided to put the dependency here as well.

the dependency was removed

arkadius · 2024-12-16T21:44:07Z

build.sbt

@@ -2051,24 +2074,18 @@ lazy val designer = (project in file("designer/server"))
    processReports,
    security,
    deploymentManagerApi,
+    componentsApi,
+    requestResponseComponentsApi,


Designer and Runtime should define the API of components they are supposed to work with. And with these dependencies we express it.

Yeah, but I thought that componentsApi is only necessary. These below have some engine-specific agreement that designer shouldn't be aware of

Yes, so do I. But I got this when we don't add it:

I wonder if engine<->designer separation is as good as the models/managers<->desinger one?

the dependencies were removed

github-actions · 2025-01-10T11:14:48Z

created: #7410
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

mk-software-pl added 9 commits November 12, 2024 10:53

wip

8adbf62

wip

aae2697

Merge remote-tracking branch 'origin/staging' into change/NU-1777

5016531

added: test

0aab679

Merge remote-tracking branch 'origin/staging' into change/NU-1777

492a8a5

wip

69b16bd

wip

76aa445

wip

3b5e463

wip

679ba89

github-actions bot added the docs label Dec 13, 2024

coutoPL and others added 5 commits December 13, 2024 17:39

Merge branch 'staging' into change/NU-1777

3c81ad9

Merge branch 'staging' into change/NU-1777

f05afa7

wip

bad1538

cypress tests fix

078ae59

fix

a9e6b27

github-actions bot added client client main fe ui labels Dec 14, 2024

mk-software-pl added 5 commits December 14, 2024 21:14

fix

5a06033

improvements

b110061

cleanup

ad6f1f0

fix

7d285f4

fix

807e1b9

coutoPL marked this pull request as ready for review December 16, 2024 08:58

mk-software-pl added 3 commits December 16, 2024 11:18

review fix

9a50dcf

wip

d4682e2

fix

0bd104e

arkadius reviewed Dec 16, 2024

View reviewed changes

coutoPL and others added 3 commits December 16, 2024 23:12

Merge branch 'staging' into change/NU-1777

b49d999

review fixes

37940bc

review fixes

f7695c7

mk-software-pl and others added 25 commits January 8, 2025 12:03

wip

1236a92

fix: sbt installed explicitly in the runner image in the setup stage

673a1ac

test

8ef1525

test

4598673

fix

706c66a

fix

4140164

Merge branch 'staging' into fix/explicity_install_sbt_in_pipeline

8a4225a

review fixes

5fd02c3

revert

f34a589

Merge branch 'fix/explicity_install_sbt_in_pipeline' into change/NU-1777

6cf7695

fix

864d8bc

fix

9d7a612

Merge branch 'staging' into change/NU-1777

80f6412

fix

e15ddea

fix

6a0620c

fix

eb901db

fix

0dbe366

fix

0dee027

fix

e437e09

Merge branch 'staging' into change/NU-1777

7f0ffff

fix

b07aedb

wip

1e7f29e

fix

09103b2

Merge remote-tracking branch 'origin/staging' into change/NU-1777

b2b96bd

Merge branch 'refactoring/NU-1777_nu_runtime_app' into change/NU-1777

3c9c0ff

mk-software-pl changed the base branch from staging to refactoring/NU-1777_nu_runtime_app January 10, 2025 10:38

mk-software-pl requested a review from arkadius January 10, 2025 10:38

cleanup

71346a6

github-actions bot mentioned this pull request Jan 10, 2025

Update Cypress snapshots in change/NU-1777 #7410

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NU-1777] Separate classloader for deployment managers #7335

[NU-1777] Separate classloader for deployment managers #7335

coutoPL commented Dec 13, 2024 •

edited by mk-software-pl

Loading

arkadius Dec 16, 2024

coutoPL Dec 16, 2024

arkadius Dec 17, 2024

arkadius Dec 16, 2024

arkadius Dec 16, 2024

coutoPL Dec 16, 2024

arkadius Dec 16, 2024

coutoPL Dec 16, 2024

mk-software-pl Jan 10, 2025

arkadius Dec 16, 2024

coutoPL Dec 16, 2024

arkadius Dec 16, 2024 •

edited

Loading

coutoPL Dec 16, 2024 •

edited

Loading

mk-software-pl Jan 10, 2025

github-actions bot commented Jan 10, 2025

[NU-1777] Separate classloader for deployment managers #7335

Are you sure you want to change the base?

[NU-1777] Separate classloader for deployment managers #7335

Conversation

coutoPL commented Dec 13, 2024 • edited by mk-software-pl Loading

Describe your changes

Checklist before merge

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arkadius Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

coutoPL Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2025

coutoPL commented Dec 13, 2024 •

edited by mk-software-pl

Loading

arkadius Dec 16, 2024 •

edited

Loading

coutoPL Dec 16, 2024 •

edited

Loading