ashokkumarrathore commented on PR #462:
URL: https://github.com/apache/livy/pull/462#issuecomment-4750907671
Rebased onto the latest `master` and resolved all conflicts — the PR is now
mergeable. Summary of what changed since the last review:
**Rebase / conflict resolution**
- Rebased onto current `master` (the branch was ~42 commits behind).
- Dropped changes that `master` has since made independently: the
`commons.lang3` import and the `SQLInterpreterSpec` assertion (kept `master`'s
broader version).
- Removed redundant per-module pom overrides (maven-shade-plugin,
commons-lang3, netty-all, hadoop-common) now that the parent pom manages those
versions; one of them would otherwise have downgraded maven-shade-plugin 3.5.0
→ 3.4.1.
**Version bumps (security)** — kept the higher version across `master`/PR:
kubernetes-client `6.8.1`, netty `4.1.108.Final`. Verified the code compiles
and unit tests pass against the fabric8 6.x client.
**Bug fix in `SparkApp.getNamespace`** — the previous version
unconditionally read `$SPARK_HOME/conf/spark-defaults.conf` and called `.get`
on `sparkHome()`, which threw `FileNotFoundException`/`NoSuchElementException`
and broke batch-session creation (and several existing tests) for
non-Kubernetes deployments. It now:
- resolves a namespace only on Kubernetes (empty string otherwise, no
filesystem access),
- treats a missing or key-less `spark-defaults.conf` as "no value" and
falls back to the `default` namespace,
- closes the input stream.
This also addresses @jahstreet's earlier question about whether the
namespace can be empty initially.
**Tests** — added `SparkAppSpec` covering the full namespace-resolution
precedence chain (session conf → `spark-defaults.conf` → default), plus the
non-Kubernetes and missing-`SPARK_HOME` paths. All touched suites pass locally
(`SparkAppSpec`, `BatchSessionSpec`, `SessionManagerSpec`,
`SparkKubernetesAppSpec`); scalastyle is clean.
Commits were squashed into one. @gyogal — could you kick off the check jobs
when you get a chance? Thanks!
> Note on the watched-namespace truncation idea (@jahstreet): I've left that
out of this PR to keep it focused; happy to follow up in a separate change
since picking a safe truncation point needs more thought.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
