[pull] master from apache:master#570
Merged
pull[bot] merged 1 commit intomiqdigital:masterfrom Mar 17, 2026
Merged
Conversation
…-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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )