Commit 394e245
authored
[ZEPPELIN-6400] Remove ZeppelinConfiguration dependency from zeppelin-interpreter module
## Summary
- **Move `ZeppelinConfiguration` from `zeppelin-interpreter` to `zeppelin-zengine`** so it is no longer included in the shaded interpreter JAR. This prevents the Maven shade plugin from corrupting config string literals, which caused classpath-order-dependent configuration loading failures.
- **Replace `ZeppelinConfiguration` usage in `zeppelin-interpreter` with `Properties`-based configuration** across `InterpreterLauncher`, `LifecycleManager`, `RecoveryStorage`, `DependencyResolver`, and all launcher plugins (Docker, K8s, YARN, Flink).
- **Update callers** in `zeppelin-zengine`, `zeppelin-server`, `flink`, and `markdown` interpreter modules.
- **Fix `TimeoutLifecycleManager` to parse time unit suffixes** (e.g., `"10s"`, `"1000ms"`) by adding `parseTimeValue()`. Previously, `Long.parseLong("10s")` threw `NumberFormatException`, causing the `TimeoutLifecycleManagerTest.testTimeout_2` to enter an infinite loop and hang CI for 6 hours.
- **Fix flaky `PersonalizeActionsIT.testGraphAction` Selenium test** by using `clickAndWait()` instead of `clickableWait().click()`, allowing the UI to update before assertion.
## Motivation
`ZeppelinConfiguration` in `zeppelin-interpreter` gets processed by the Maven shade plugin, which corrupts string literals (e.g., `org.apache.zeppelin` → `unshaded.org.apache.zeppelin`). This causes config keys to mismatch at runtime depending on classpath ordering. Moving it to `zeppelin-zengine` (which is not shaded) permanently eliminates this class of bugs.
As discussed by the community: *"ZeppelinConfiguration belongs to the Zeppelin server, and the Zeppelin interpreter should really only work on a HashMap with ConfigKey and ConfigValue."*
## Changes
| Area | Change |
|------|--------|
| `zeppelin-interpreter` (core) | Remove `ZeppelinConfiguration` imports; use `Properties` for config |
| `InterpreterLauncher` | `ZeppelinConfiguration zConf` → `Properties zProperties` |
| `LifecycleManager` / `RecoveryStorage` | Constructor takes `Properties` instead of `ZeppelinConfiguration` |
| `TimeoutLifecycleManager` | Add `parseTimeValue()` to handle time unit suffixes (`"10s"`, `"1000ms"`) |
| `DependencyResolver` | Accept individual config values instead of `ZeppelinConfiguration` |
| Launcher plugins (7 files) | Updated to `Properties`-based API |
| `zeppelin-zengine` | `PluginManager` passes derived values (absolute paths) via Properties |
| `ZeppelinConfiguration.java` | Moved from `zeppelin-interpreter` → `zeppelin-zengine` |
| `PersonalizeActionsIT` | Fix flaky `testGraphAction` by waiting for UI update after click |
## Future Work
Some logic was duplicated during this refactoring to keep `zeppelin-interpreter` independent of `ZeppelinConfiguration`:
- `TimeoutLifecycleManager.parseTimeValue()` duplicates `ZeppelinConfiguration.timeUnitToMill()` — both parse time strings like `"10s"` or `"1000ms"` via `Duration.parse("PT" + value)`. A shared utility in `zeppelin-common` could consolidate this in the future.
- Config key strings (e.g., `"zeppelin.interpreter.lifecyclemanager.timeout.threshold"`) are now hardcoded as plain strings in `zeppelin-interpreter` rather than referencing `ConfVars` enum constants. If config key management becomes an issue, a lightweight key constants class could be introduced.
## Test Plan
- [x] CI: `core.yml` - Core module tests (including `TimeoutLifecycleManagerTest`)
- [x] CI: `core.yml` - Interpreter tests (Spark, Flink)
- [x] CI: `frontend.yml` - E2E tests (Playwright + Selenium)
- [x] CI: `quick.yml` - RAT license check
- [x] Verify shaded JAR does not contain `ZeppelinConfiguration`
Closes #5167 from jongyoul/ZEPPELIN-6400-remove-zepconf-from-interpreter.
Signed-off-by: Jongyoul Lee <jongyoul@gmail.com>1 parent 5c35ad9 commit 394e245
37 files changed
Lines changed: 130 additions & 156 deletions
File tree
- flink/flink-scala-2.12/src/main
- java/org/apache/zeppelin/flink
- scala/org/apache/zeppelin/flink
- markdown/src
- main/java/org/apache/zeppelin/markdown
- test/java/org/apache/zeppelin/markdown
- zeppelin-integration/src/test/java/org/apache/zeppelin/integration
- zeppelin-interpreter/src
- main/java/org/apache/zeppelin
- dep
- interpreter
- lifecycle
- remote
- scheduler
- test/java/org/apache/zeppelin
- conf
- dep
- helium
- interpreter/remote
- zeppelin-server/src
- main/java/org/apache/zeppelin/service
- test/java/org/apache/zeppelin/service
- zeppelin-zengine/src
- main/java/org/apache/zeppelin
- conf
- interpreter
- install
- launcher
- recovery
- remote
- test/java/org/apache/zeppelin/interpreter
- launcher
Lines changed: 2 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
26 | 25 | | |
27 | 26 | | |
28 | 27 | | |
| |||
91 | 90 | | |
92 | 91 | | |
93 | 92 | | |
94 | | - | |
95 | | - | |
| 93 | + | |
| 94 | + | |
96 | 95 | | |
97 | 96 | | |
98 | 97 | | |
| |||
Lines changed: 2 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
31 | 30 | | |
32 | 31 | | |
33 | | - | |
34 | | - | |
35 | | - | |
| 32 | + | |
| 33 | + | |
36 | 34 | | |
37 | 35 | | |
38 | 36 | | |
| |||
Lines changed: 7 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
48 | 47 | | |
49 | 48 | | |
50 | 49 | | |
| |||
66 | 65 | | |
67 | 66 | | |
68 | 67 | | |
69 | | - | |
70 | | - | |
| 68 | + | |
71 | 69 | | |
72 | 70 | | |
73 | 71 | | |
| |||
800 | 798 | | |
801 | 799 | | |
802 | 800 | | |
803 | | - | |
| 801 | + | |
| 802 | + | |
| 803 | + | |
| 804 | + | |
| 805 | + | |
| 806 | + | |
804 | 807 | | |
805 | 808 | | |
806 | 809 | | |
| |||
Lines changed: 5 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
35 | 34 | | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
48 | 50 | | |
49 | 51 | | |
50 | 52 | | |
| |||
55 | 57 | | |
56 | 58 | | |
57 | 59 | | |
58 | | - | |
| 60 | + | |
59 | 61 | | |
60 | 62 | | |
61 | 63 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
77 | | - | |
| 77 | + | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| |||
Lines changed: 0 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
21 | 20 | | |
22 | 21 | | |
23 | 22 | | |
| |||
44 | 43 | | |
45 | 44 | | |
46 | 45 | | |
47 | | - | |
48 | 46 | | |
49 | 47 | | |
50 | 48 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
213 | | - | |
214 | | - | |
| 213 | + | |
| 214 | + | |
215 | 215 | | |
216 | 216 | | |
217 | 217 | | |
| |||
Lines changed: 12 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
| |||
49 | 48 | | |
50 | 49 | | |
51 | 50 | | |
52 | | - | |
53 | | - | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
54 | 54 | | |
55 | | - | |
56 | | - | |
57 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
58 | 60 | | |
59 | | - | |
| 61 | + | |
60 | 62 | | |
61 | 63 | | |
62 | 64 | | |
63 | | - | |
| 65 | + | |
64 | 66 | | |
65 | 67 | | |
66 | 68 | | |
67 | | - | |
68 | | - | |
| 69 | + | |
69 | 70 | | |
70 | 71 | | |
71 | | - | |
| 72 | + | |
72 | 73 | | |
73 | 74 | | |
74 | 75 | | |
| |||
Lines changed: 2 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
22 | 21 | | |
23 | 22 | | |
24 | 23 | | |
| |||
83 | 82 | | |
84 | 83 | | |
85 | 84 | | |
86 | | - | |
| 85 | + | |
87 | 86 | | |
88 | 87 | | |
89 | | - | |
| 88 | + | |
90 | 89 | | |
91 | 90 | | |
92 | 91 | | |
| |||
Lines changed: 5 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
31 | 30 | | |
32 | 31 | | |
33 | 32 | | |
| |||
56 | 55 | | |
57 | 56 | | |
58 | 57 | | |
59 | | - | |
60 | | - | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
61 | 61 | | |
62 | 62 | | |
63 | | - | |
64 | | - | |
| 63 | + | |
| 64 | + | |
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
| |||
0 commit comments