Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[admin-tool] Enable venice-admin-tool to parse JSON map arguments from command line #737

Merged
merged 5 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ private static void stringMapParam(
Arg param,
Consumer<Map<String, String>> setter,
Set<Arg> argSet) {
genericParam(cmd, param, s -> Utils.parseCommaSeparatedStringMapFromString(s, param.toString()), setter, argSet);
genericParam(cmd, param, s -> Utils.parseJsonMapFromString(s, param.toString()), setter, argSet);
}

private static void stringSetParam(CommandLine cmd, Arg param, Consumer<Set<String>> setter, Set<Arg> argSet) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testAdminUpdateStoreArg() throws ParseException, IOException {
final String K1 = "k1", V1 = "v1", K2 = "k2", V2 = "v2", K3 = "k3", V3 = "v3";
String[] args = { "--update-store", "--url", "http://localhost:7036", "--cluster", "test-cluster", "--store",
"testStore", "--rmd-chunking-enabled", "true", "--partitioner-params",
K1 + "=" + V1 + "," + K2 + "=" + V2 + "," + K3 + "=" + V3 };
"{\"" + K1 + "\":\"" + V1 + "\",\"" + K2 + "\":\"" + V2 + "\",\"" + K3 + "\":\"" + V3 + "\"}" };

CommandLine commandLine = AdminTool.getCommandLine(args);
UpdateStoreQueryParams params = AdminTool.getUpdateStoreQueryParams(commandLine);
Expand Down Expand Up @@ -293,7 +293,7 @@ public void testAdminConfigureView() throws ParseException, IOException {
// Case 1: Happy path to setup a view.
String[] args = { "--configure-store-view", "--url", "http://localhost:7036", "--cluster", "test-cluster",
"--store", "testStore", "--view-name", "testView", "--view-class", ChangeCaptureView.class.getCanonicalName(),
"--view-params", K1 + "=" + V1 + "," + K2 + "=" + V2 };
"--view-params", "{\"" + K1 + "\":\"" + V1 + "\",\"" + K2 + "\":\"" + V2 + "\"}" };

CommandLine commandLine = AdminTool.getCommandLine(args);
UpdateStoreQueryParams params = AdminTool.getConfigureStoreViewQueryParams(commandLine);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static com.linkedin.venice.HttpConstants.LOCALHOST;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.linkedin.avroutil1.compatibility.AvroCompatibilityHelper;
import com.linkedin.venice.controllerapi.ControllerResponse;
import com.linkedin.venice.exceptions.ConfigurationException;
Expand Down Expand Up @@ -306,18 +308,20 @@ public static boolean parseBooleanFromString(String value, String fieldName) {
}

/**
* For String-String key-value map config, we expect the command-line interface users to use "key1=value1,key2=value2,..."
* For String-String key-value map config, we expect the command-line interface users to use JSON
* format to represent it. This method deserialize it to String-String map.
*/
public static Map<String, String> parseCommaSeparatedStringMapFromString(String value, String fieldName) {
public static Map<String, String> parseJsonMapFromString(String value, String fieldName) {
try {
nisargthakkar marked this conversation as resolved.
Show resolved Hide resolved
Map<String, String> map = new HashMap<>();
if (!value.isEmpty()) {
Arrays.stream(value.split(",")).map(s -> s.split("=")).forEach(strings -> map.put(strings[0], strings[1]));
ObjectMapper objectMapper = ObjectMapperFactory.getInstance();
return objectMapper.readValue(value, new TypeReference<Map<String, String>>() {
});
}
return map;
} catch (Exception e) {
throw new VeniceException(fieldName + " must be key value pairs separated by comma, but value: " + value);
} catch (IOException jsonException) {
throw new VeniceException(fieldName + " must be a valid JSON object, but value: " + value);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,16 @@ public void testIterateOnMapOfLists() throws Exception {

@Test
public void testParseMap() {
assertEquals(Utils.parseCommaSeparatedStringMapFromString("", "test_field").size(), 0);
Map nonEmptyMap = Utils.parseCommaSeparatedStringMapFromString("a=b", "test_field");
Map expectedMap = new HashMap<>();
expectedMap.put("a", "b");
assertEquals(nonEmptyMap, expectedMap);

VeniceException e = expectThrows(
VeniceException.class,
() -> Utils.parseCommaSeparatedStringMapFromString("invalid_value", "test_field"));
assertTrue(e.getMessage().contains("must be key value pairs separated by comma"));
assertEquals(Utils.parseJsonMapFromString("", "test_field").size(), 0);
Map validMap = Utils.parseJsonMapFromString("{\"a\":\"b\"}", "test_field");
assertEquals(validMap, expectedMap);

VeniceException e = expectThrows(VeniceException.class, () -> Utils.parseJsonMapFromString("a=b", "test_field"));
assertTrue(e.getMessage().contains("must be a valid JSON object"));

}

@Test
Expand Down