Skip to content

Commit

Permalink
[ZEPPELIN-6131] Update Interpreter to Store Values as Integers When A…
Browse files Browse the repository at this point in the history
…pplicable

### What is this PR for?
There was an issue where the port was treated as a string and included `.0` when modifying the port of the Elasticsearch interpreter.

### What type of PR is it?
Bug Fix

### Todos
* [x] - Modified HttpBasedClient.java file

### What is the Jira issue?
* https://issues.apache.org/jira/projects/ZEPPELIN/issues/ZEPPELIN-6131

### How should this be tested?
* Zeppelin project build and run
```bash
./mvnw clean package -DskipTests
```

* Add a test index and test data to Elasticsearch
```bash
# Create an index
curl -X PUT "http://localhost:9200/customer?pretty"

# Create a document
curl -X \
PUT "localhost:9200/customer/_doc/1" \
-H 'Content-Type: application/json' \
-d'
{
  "field_1": "value_1"
}'
```

* Modify the Elasticsearch interpreter type and port
elasticsearch.client.type: transport -> http
elasticsearch.port: 9300 -> 9200

* Previous configuration
<img width="1228" alt="image" src="https://github.com/user-attachments/assets/aab69016-b982-4817-8d72-68186fdd45b8">

* New configuration
<img width="1219" alt="image" src="https://github.com/user-attachments/assets/a9c44503-1ed1-43c0-89f9-19b92a1ac1e0">

* Create a Zeppelin notebook and execute a paragraph
```bash
%elasticsearch
get /customer/_doc/1
```

* Result before modification - Error occurred
<img width="942" alt="image" src="https://github.com/user-attachments/assets/00d5db2f-e1dd-46bf-96d9-eb2eb1d88072">

```bash
org.apache.zeppelin.interpreter.InterpreterException: java.lang.NumberFormatException: For input string: "9200.0"
	at org.apache.zeppelin.interpreter.LazyOpenInterpreter.open(LazyOpenInterpreter.java:77)
	at org.apache.zeppelin.interpreter.remote.RemoteInterpreterServer$InterpretJob.jobRun(RemoteInterpreterServer.java:808)
	at org.apache.zeppelin.interpreter.remote.RemoteInterpreterServer$InterpretJob.jobRun(RemoteInterpreterServer.java:716)
	at org.apache.zeppelin.scheduler.Job.run(Job.java:187)
	at org.apache.zeppelin.scheduler.AbstractScheduler.runJob(AbstractScheduler.java:136)
	at org.apache.zeppelin.scheduler.FIFOScheduler.lambda$runJobInScheduler$0(FIFOScheduler.java:42)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.NumberFormatException: For input string: "9200.0"
	at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.base/java.lang.Integer.parseInt(Integer.java:652)
	at java.base/java.lang.Integer.parseInt(Integer.java:770)
	at org.apache.zeppelin.elasticsearch.client.HttpBasedClient.<init>(HttpBasedClient.java:67)
	at org.apache.zeppelin.elasticsearch.ElasticsearchInterpreter.open(ElasticsearchInterpreter.java:136)
	at org.apache.zeppelin.interpreter.LazyOpenInterpreter.open(LazyOpenInterpreter.java:71)
	... 8 more
```

* Result after modification
<img width="500" alt="image" src="https://github.com/user-attachments/assets/2634216c-ed42-4a1e-8c6d-d3bb378365fa">

### Screenshots (if appropriate)
* Refer to the above content

### Questions:
* Does the license files need to update? No.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.


Closes #4878 from ParkGyeongTae/fix-elasticsearch-port-string-handling.

Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
  • Loading branch information
ParkGyeongTae authored Nov 12, 2024
1 parent ce04862 commit 4a7fdb1
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,55 @@ public UpdateInterpreterSettingRequest(Map<String, InterpreterProperty> properti
this.option = option;
}

/**
* Retrieves the properties of the interpreter and, if the property type is "number"
* and its value is a double that represents a whole number, converts that value to an integer.
*
* @return A map of the interpreter's properties with possible modifications to the numeric properties.
*/
public Map<String, InterpreterProperty> getProperties() {
properties.forEach((key, property) -> {
if (isNumberType(property) && isWholeNumberDouble(property.getValue())) {
convertDoubleToInt(property);
}
});
return properties;
}

/**
* Checks if the property type is "number".
*
* @param property The InterpreterProperty to check.
* @return true if the property type is "number", false otherwise.
*/
private boolean isNumberType(InterpreterProperty property) {
return "number".equals(property.getType());
}

/**
* Checks if the given value is a Double and represents a whole number.
*
* @param value The object to check.
* @return true if the value is a Double and a whole number, false otherwise.
*/
private boolean isWholeNumberDouble(Object value) {
if (value instanceof Double) {
Double doubleValue = (Double) value;
return doubleValue == Math.floor(doubleValue);
}
return false;
}

/**
* Converts the value of the given property from a Double to an Integer if the Double represents a whole number.
*
* @param property The InterpreterProperty whose value will be converted.
*/
private void convertDoubleToInt(InterpreterProperty property) {
Double doubleValue = (Double) property.getValue();
property.setValue(doubleValue.intValue());
}

public List<Dependency> getDependencies() {
return dependencies;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.zeppelin.rest.message;

import org.apache.zeppelin.dep.Dependency;
import org.apache.zeppelin.interpreter.InterpreterOption;
import org.apache.zeppelin.interpreter.InterpreterProperty;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

/**
* Unit test class for the UpdateInterpreterSettingRequest.
* This class tests the behavior of UpdateInterpreterSettingRequest methods,
* especially focusing on property type handling and value conversions.
*/
class UpdateInterpreterSettingRequestTest {

private Map<String, InterpreterProperty> properties;
private List<Dependency> dependencies;
private InterpreterOption option;
private UpdateInterpreterSettingRequest request;

/**
* Setup method that initializes test fixtures before each test.
* Mocks dependencies and option objects to isolate UpdateInterpreterSettingRequest behavior.
*/
@BeforeEach
void setUp() {
properties = new HashMap<>();
dependencies = Collections.emptyList();
option = mock(InterpreterOption.class);
request = new UpdateInterpreterSettingRequest(properties, dependencies, option);
}

/**
* Tests getProperties method to verify that properties with "number" type
* and whole-number Double values are correctly converted to Integer.
* Verifies that only whole-number Doubles are converted and non-integer Doubles remain unchanged.
*/
@Test
void testGetPropertiesWithWholeNumberDoubleConversion() {
InterpreterProperty property1 = mock(InterpreterProperty.class);
when(property1.getType()).thenReturn("number");
when(property1.getValue()).thenReturn(5.0);

InterpreterProperty property2 = mock(InterpreterProperty.class);
when(property2.getType()).thenReturn("number");
when(property2.getValue()).thenReturn(5.5);

properties.put("property1", property1);
properties.put("property2", property2);

Map<String, InterpreterProperty> resultProperties = request.getProperties();

verify(property1).setValue(5);
verify(property2, never()).setValue(any());
assertEquals(properties, resultProperties);
}

/**
* Tests getProperties method when the property type is not "number".
* Verifies that no conversion is performed on non-number types.
*/
@Test
void testGetPropertiesWithoutConversion() {
InterpreterProperty property = mock(InterpreterProperty.class);
when(property.getType()).thenReturn("string");
when(property.getValue()).thenReturn("test");

properties.put("property", property);

Map<String, InterpreterProperty> resultProperties = request.getProperties();

verify(property, never()).setValue(any());
assertEquals(properties, resultProperties);
}

/**
* Tests getDependencies method to confirm that it returns the correct dependencies list.
*/
@Test
void testGetDependencies() {
assertEquals(dependencies, request.getDependencies());
}

/**
* Tests getOption method to confirm that it returns the correct interpreter option.
*/
@Test
void testGetOption() {
assertEquals(option, request.getOption());
}
}

0 comments on commit 4a7fdb1

Please sign in to comment.