Skip to content

Commit 4297661

Browse files
authored
[ZEPPELIN-6308] Extract hardcoded py4j values to constants
### What is this PR for? This PR extracts hardcoded py4j values to constants in Python interpreters to improve maintainability. It creates a new PythonConstants class to centralize py4j version and file path constants, replacing hardcoded strings across multiple Python interpreter classes. This addresses the TODO comment requesting to avoid hardcoded py4j values. ### What type of PR is it? Refactoring ### Todos * [x] - Create PythonConstants class with py4j constants * [x] - Replace hardcoded py4j values in IPythonInterpreter * [x] - Replace hardcoded py4j values in PythonInterpreter * [x] - Replace hardcoded py4j values in PythonDockerInterpreter ### What is the Jira issue? [ZEPPELIN-6308](https://issues.apache.org/jira/browse/ZEPPELIN-6308) ### How should this be tested? ### Screenshots (if appropriate) ### Questions: * Does the license files need to update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Closes #5056 from celinayk/ZEPPELIN-6308. Signed-off-by: ParkGyeongTae <gyeongtae@apache.org>
1 parent 85b7550 commit 4297661

File tree

4 files changed

+44
-6
lines changed

4 files changed

+44
-6
lines changed

python/src/main/java/org/apache/zeppelin/python/IPythonInterpreter.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,9 @@ private void initPythonInterpreter(String gatewayHost, int gatewayPort) throws I
178178
protected Map<String, String> setupKernelEnv() throws IOException {
179179
Map<String, String> envs = super.setupKernelEnv();
180180
if (useBuiltinPy4j) {
181-
//TODO(zjffdu) don't do hard code on py4j here
182-
File py4jDestFile = new File(kernelWorkDir, "py4j-src-0.10.9.7.zip");
181+
File py4jDestFile = new File(kernelWorkDir, PythonConstants.PY4J_ZIP_FILENAME);
183182
FileUtils.copyURLToFile(getClass().getClassLoader().getResource(
184-
"python/py4j-src-0.10.9.7.zip"), py4jDestFile);
183+
PythonConstants.PY4J_RESOURCE_PATH), py4jDestFile);
185184
if (additionalPythonPath != null) {
186185
// put the py4j at the end, because additionalPythonPath may already contain py4j.
187186
// e.g. IPySparkInterpreter
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.zeppelin.python;
19+
20+
/**
21+
* Constants for Python interpreters
22+
*/
23+
public class PythonConstants {
24+
25+
// Py4j constants
26+
public static final String PY4J_VERSION = "0.10.9.7";
27+
public static final String PY4J_ZIP_FILENAME = "py4j-src-" + PY4J_VERSION + ".zip";
28+
public static final String PY4J_RESOURCE_PATH = "python/" + PY4J_ZIP_FILENAME;
29+
30+
private PythonConstants() {
31+
// Utility class, prevent instantiation
32+
}
33+
}
34+

python/src/main/java/org/apache/zeppelin/python/PythonDockerInterpreter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ public InterpreterResult interpret(String st, InterpreterContext context)
9191
":/_zeppelin ";
9292

9393
// set PYTHONPATH
94-
String pythonPath = ".:/_python_workdir/py4j-src-0.10.9.7.zip:/_python_workdir";
94+
String pythonPath = ".:/_python_workdir/" +
95+
PythonConstants.PY4J_ZIP_FILENAME + ":/_python_workdir";
9596

9697
setPythonCommand("docker run -i --rm " +
9798
mountPythonScript +

python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,10 @@ private void createPythonScript() throws IOException {
186186
copyResourceToPythonWorkDir("python/zeppelin_context.py", "zeppelin_context.py");
187187
copyResourceToPythonWorkDir("python/backend_zinline.py", "backend_zinline.py");
188188
copyResourceToPythonWorkDir("python/mpl_config.py", "mpl_config.py");
189-
copyResourceToPythonWorkDir("python/py4j-src-0.10.9.7.zip", "py4j-src-0.10.9.7.zip");
189+
copyResourceToPythonWorkDir(
190+
PythonConstants.PY4J_RESOURCE_PATH,
191+
PythonConstants.PY4J_ZIP_FILENAME);
192+
190193
}
191194

192195
protected boolean useIPython() {
@@ -212,7 +215,8 @@ protected Map<String, String> setupPythonEnv() throws IOException {
212215
Map<String, String> env = EnvironmentUtils.getProcEnvironment();
213216
appendToPythonPath(env, pythonWorkDir.getAbsolutePath());
214217
if (useBuiltinPy4j) {
215-
appendToPythonPath(env, pythonWorkDir.getAbsolutePath() + "/py4j-src-0.10.9.7.zip");
218+
appendToPythonPath(env,
219+
pythonWorkDir.getAbsolutePath() + "/" + PythonConstants.PY4J_ZIP_FILENAME);
216220
}
217221
LOGGER.info("PYTHONPATH: {}", env.get("PYTHONPATH"));
218222
return env;

0 commit comments

Comments
 (0)