Re: [PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
zeroflag merged PR #835: URL: https://github.com/apache/knox/pull/835 -- 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: dev-unsubscr...@knox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
zeroflag commented on code in PR #835:
URL: https://github.com/apache/knox/pull/835#discussion_r1469398237
##
gateway-server/src/test/java/org/apache/knox/gateway/util/DescriptorGeneratorTest.java:
##
@@ -0,0 +1,55 @@
+package org.apache.knox.gateway.util;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.nio.charset.Charset;
+import java.util.Arrays;
+import java.util.List;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.knox.gateway.model.DescriptorConfiguration;
+import org.apache.knox.gateway.model.Topology;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class DescriptorGeneratorTest {
+ private static final ObjectMapper mapper = new ObjectMapper();
+ private static final String TEST_DESC_1 = "test_desc1.json";
+ private static final String TEST_PROV_1 = "test_prov1.json";
+ private static final String IMPALA_UI = "IMPALAUI";
+ private static final List URLS =
+ Arrays.asList("http://amagyar-1.test.site:25000/;,
"http://amagyar-2.test.site:25000;);
+ @Rule
+ public TemporaryFolder folder= new TemporaryFolder();
Review Comment:
This is not for creating an input file, but for the resulting output file.
The generrated descriptor will be placed into this temp folder (and junit will
clean it automatically).
--
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: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
smolnar82 commented on code in PR #835:
URL: https://github.com/apache/knox/pull/835#discussion_r1467330890
##
gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java:
##
@@ -2392,6 +2413,55 @@ public String getUsage() {
}
+ public class GenerateDescriptorCommand extends Command {
+
+public static final String USAGE =
+"generate-descriptor --service-urls-file \"path/to/urls.txt\"
--service-name SERVICE_NAME \n" +
+"--provider-name my-provider.json --descriptor-name
my-descriptor.json \n" +
+"[--output-dir /path/to/output_dir] \n" +
+"[--force] \n";
+public static final String DESC =
+"Create Knox topology descriptor file for one service\n"
++ "Options are as follows: \n"
++ "--service-urls-file (required) path to a text file
containing service urls \n"
++ "--service-name (required) the name of the service, such
as WEBHDFS, IMPALAUI or HIVE \n"
++ "--descriptor-name (required) name of descriptor to be
created \n"
++ "--provider-name (required) name of the referenced
shared provider \n"
++ "--output-dir (optional) output directory to save the
descriptor file \n"
Review Comment:
This is an optional field. The default value is missing from the description.
--
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: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
smolnar82 commented on code in PR #835:
URL: https://github.com/apache/knox/pull/835#discussion_r1467334234
##
gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java:
##
@@ -2392,6 +2413,55 @@ public String getUsage() {
}
+ public class GenerateDescriptorCommand extends Command {
+
+public static final String USAGE =
+"generate-descriptor --service-urls-file \"path/to/urls.txt\"
--service-name SERVICE_NAME \n" +
+"--provider-name my-provider.json --descriptor-name
my-descriptor.json \n" +
+"[--output-dir /path/to/output_dir] \n" +
+"[--force] \n";
+public static final String DESC =
+"Create Knox topology descriptor file for one service\n"
++ "Options are as follows: \n"
++ "--service-urls-file (required) path to a text file
containing service urls \n"
++ "--service-name (required) the name of the service, such
as WEBHDFS, IMPALAUI or HIVE \n"
++ "--descriptor-name (required) name of descriptor to be
created \n"
++ "--provider-name (required) name of the referenced
shared provider \n"
++ "--output-dir (optional) output directory to save the
descriptor file \n"
++ "--force (optional) force rewriting of existing files,
if not used, command will fail when the configs files with same name already
exist. \n";
+
+@Override
+public void execute() throws Exception {
+ if (StringUtils.isBlank(FilenameUtils.getExtension(providerName))
+ ||
StringUtils.isBlank(FilenameUtils.getExtension(descriptorName))) {
+throw new IllegalArgumentException("JSON extension is required for
provider and descriptor file names");
+ }
+ if (StringUtils.isBlank(urlsFilePath) ) {
+throw new IllegalArgumentException("Missing --service-urls-file");
+ }
+ if (!new File(urlsFilePath).isFile()) {
+throw new IllegalArgumentException(urlsFilePath + " does not exist");
+ }
+ if (StringUtils.isBlank(serviceName)) {
+throw new IllegalArgumentException("Missing --service-name");
+ }
+ File outputDir = StringUtils.isBlank(KnoxCLI.this.outputDir) ? new
File(".") : new File(KnoxCLI.this.outputDir);
+
+ DescriptorGenerator generator =
+ new DescriptorGenerator(descriptorName, providerName,
serviceName, ServiceUrls.fromFile(new File(urlsFilePath)));
+ generator.saveDescriptor(
Review Comment:
I meant the `generator.saveDescriptor(...)` call, just be make it clear :)
--
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: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
smolnar82 commented on code in PR #835:
URL: https://github.com/apache/knox/pull/835#discussion_r1467326277
##
gateway-server/src/test/java/org/apache/knox/gateway/util/DescriptorGeneratorTest.java:
##
@@ -0,0 +1,55 @@
+package org.apache.knox.gateway.util;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.nio.charset.Charset;
+import java.util.Arrays;
+import java.util.List;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.knox.gateway.model.DescriptorConfiguration;
+import org.apache.knox.gateway.model.Topology;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class DescriptorGeneratorTest {
+ private static final ObjectMapper mapper = new ObjectMapper();
+ private static final String TEST_DESC_1 = "test_desc1.json";
+ private static final String TEST_PROV_1 = "test_prov1.json";
+ private static final String IMPALA_UI = "IMPALAUI";
+ private static final List URLS =
+ Arrays.asList("http://amagyar-1.test.site:25000/;,
"http://amagyar-2.test.site:25000;);
+ @Rule
+ public TemporaryFolder folder= new TemporaryFolder();
Review Comment:
Instead of making this a temporary file, we could create a static file in
`src/test/resources` and have the code read that file. Since this is not a
dynamic one, I think it would make more sense and we would avoid creating a
temporary folder every time this test is running (the read I/O operation is
unavoidable, but creation can be).
##
gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java:
##
@@ -2392,6 +2413,55 @@ public String getUsage() {
}
+ public class GenerateDescriptorCommand extends Command {
+
+public static final String USAGE =
+"generate-descriptor --service-urls-file \"path/to/urls.txt\"
--service-name SERVICE_NAME \n" +
+"--provider-name my-provider.json --descriptor-name
my-descriptor.json \n" +
+"[--output-dir /path/to/output_dir] \n" +
+"[--force] \n";
+public static final String DESC =
+"Create Knox topology descriptor file for one service\n"
++ "Options are as follows: \n"
++ "--service-urls-file (required) path to a text file
containing service urls \n"
++ "--service-name (required) the name of the service, such
as WEBHDFS, IMPALAUI or HIVE \n"
++ "--descriptor-name (required) name of descriptor to be
created \n"
++ "--provider-name (required) name of the referenced
shared provider \n"
++ "--output-dir (optional) output directory to save the
descriptor file \n"
++ "--force (optional) force rewriting of existing files,
if not used, command will fail when the configs files with same name already
exist. \n";
+
+@Override
+public void execute() throws Exception {
+ if (StringUtils.isBlank(FilenameUtils.getExtension(providerName))
+ ||
StringUtils.isBlank(FilenameUtils.getExtension(descriptorName))) {
+throw new IllegalArgumentException("JSON extension is required for
provider and descriptor file names");
+ }
+ if (StringUtils.isBlank(urlsFilePath) ) {
+throw new IllegalArgumentException("Missing --service-urls-file");
+ }
+ if (!new File(urlsFilePath).isFile()) {
+throw new IllegalArgumentException(urlsFilePath + " does not exist");
+ }
+ if (StringUtils.isBlank(serviceName)) {
+throw new IllegalArgumentException("Missing --service-name");
+ }
Review Comment:
All of these verifications could be extracted to a private method for a
better reading experience.
##
gateway-server/src/test/java/org/apache/knox/gateway/util/DescriptorGeneratorTest.java:
##
@@ -0,0 +1,55 @@
+package org.apache.knox.gateway.util;
Review Comment:
Missing header? The build should fail anyway...
##
gateway-server/src/main/java/org/apache/knox/gateway/util/DescriptorGenerator.java:
##
@@ -0,0 +1,53 @@
+package org.apache.knox.gateway.util;
Review Comment:
Missing header? The build should fail anyway...
##
gateway-server/src/main/java/org/apache/knox/gateway/util/ServiceUrls.java:
##
@@ -0,0 +1,36 @@
+package org.apache.knox.gateway.util;
Review Comment:
Missing header? The build should fail anyway...
##
gateway-server/src/main/java/org/apache/knox/gateway/util/KnoxCLI.java:
##
@@ -2392,6 +2413,55 @@ public String getUsage() {
}
+ public class GenerateDescriptorCommand extends Command {
+
+public static final String USAGE =
+"generate-descriptor --service-urls-file \"path/to/urls.txt\"
--service-name SERVICE_NAME \n" +
+"--provider-name
[PR] KNOX-3002 - KnoxCLI command for generating descriptor for a role type from a list of hosts [knox]
zeroflag opened a new pull request, #835:
URL: https://github.com/apache/knox/pull/835
## What changes were proposed in this pull request?
Adding a command to knoxcli that can generate a topology descriptor from a
list of URLs.
Usage:
```
Create Knox topology descriptor file for one service
Options are as follows:
--service-urls-file (required) path to a text file containing service urls
--service-name (required) the name of the service, such as WEBHDFS, IMPALAUI
or HIVE
--descriptor-name (required) name of descriptor to be created
--provider-name (required) name of the referenced shared provider
--output-dir (optional) output directory to save the descriptor file
--force (optional) force rewriting of existing files, if not used, command
will fail when the configs files with same name already exist.
```
## How was this patch tested?
```bash
cat /tmp/urls.txt
http://url1.site:5000
http://url2.site:5000
```
```bash
$ bin/knoxcli.sh generate-descriptor --service-urls-file /tmp/urls.txt
--service-name IMPALAUI --provider-name pam.json --descriptor-name impalads.json
Descriptor impalads.json was successfully saved to
/Users/attilamagyar/development/test/.
```
```bash
cat impalads.txt
{
"name" : "impalads",
"provider-config-ref" : "pam",
"services" : [ {
"params" : { },
"name" : "IMPALAUI",
"urls" : [ "http://url1.site:5000;, "http://url2.site:5000; ]
} ]
}
```
--
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: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
