[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2566


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170421681
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
+File[] jarFiles = dir.listFiles((dir1, name) -> 
name.endsWith(".jar"));
+
+if (jarFiles != null) {
--- End diff --

Yes I also feel better to throw an exception. Throwing exception will not 
break Nimbus instance too.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170419284
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1504,36 +1505,53 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Yaml yaml = new Yaml(new SafeConstructor());
 Map defaultsConf = null;
 Map stormConf = null;
+
+// Based on how Java handles the classpath
+// 
https://docs.oracle.com/javase/8/docs/technotes/tools/unix/classpath.html
 for (String part: cp) {
 File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
+
+if (f.getName().equals("*")) {
+// wildcard is given in file
+// in java classpath, '*' is expanded to all jar/JAR files 
in the directory
+File dir = f.getParentFile();
+if (dir == null) {
+// it happens when part is just '*' rather than 
denoting some directory
+dir = new File(".");
 }
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+
+File[] jarFiles = dir.listFiles((dir1, name) -> 
name.endsWith(".jar") || name.endsWith(".JAR"));
+
+if (jarFiles != null) {
+for (File jarFile : jarFiles) {
+JarConfigReader jarConfigReader = new 
JarConfigReader(yaml, defaultsConf, stormConf, jarFile).readJar();
+defaultsConf = jarConfigReader.getDefaultsConf();
+stormConf = jarConfigReader.getStormConf();
+}
 }
 } else {
--- End diff --

Nit: Looks like this block doesn't need to be at another level, I think the 
method could be written as if ... else if ... else if ...


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418714
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1588,60 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader readZip() throws IOException {
+try (ZipFile zipFile = new ZipFile(f)) {
+readArchive(zipFile);
+}
+return this;
+}
+
+public JarConfigReader readJar() throws IOException {
+try (JarFile jarFile = new JarFile(f)) {
+readArchive(jarFile);
+}
+return this;
+}
+
+private void readArchive(ZipFile zipFile) throws IOException {
+Enumeration zipEnums = zipFile.entries();
+while (zipEnums.hasMoreElements()) {
+ZipEntry entry = zipEnums.nextElement();
+if (!entry.isDirectory()) {
+if (defaultsConf == null && 
entry.getName().equals("defaults.yaml")) {
+try (InputStream in = 
zipFile.getInputStream(entry); InputStreamReader isr = new 
InputStreamReader(in)) {
--- End diff --

Addressed.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418654
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1574,49 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader invoke() throws IOException {
--- End diff --

Looks good, thanks. Not sure if it makes a difference to open the jarfile 
with JarFile rather than ZipFile, but it can't hurt.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418624
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
+File[] jarFiles = dir.listFiles((dir1, name) -> 
name.endsWith(".jar"));
+
+if (jarFiles != null) {
--- End diff --

I'm not sure. If jarFiles is null, it looks like we'll end up returning an 
empty config, so I think downstream code will probably break. If that's the 
case, I'd prefer throwing an exception here so it's obvious what went wrong. 
Whether we want to throw a custom exception or just remove the null check and 
get an NPE in the following line makes no difference to me.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418572
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
--- End diff --

Thanks, looks good


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418510
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1588,60 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader readZip() throws IOException {
+try (ZipFile zipFile = new ZipFile(f)) {
+readArchive(zipFile);
+}
+return this;
+}
+
+public JarConfigReader readJar() throws IOException {
+try (JarFile jarFile = new JarFile(f)) {
+readArchive(jarFile);
+}
+return this;
+}
+
+private void readArchive(ZipFile zipFile) throws IOException {
+Enumeration zipEnums = zipFile.entries();
+while (zipEnums.hasMoreElements()) {
+ZipEntry entry = zipEnums.nextElement();
+if (!entry.isDirectory()) {
+if (defaultsConf == null && 
entry.getName().equals("defaults.yaml")) {
+try (InputStream in = 
zipFile.getInputStream(entry); InputStreamReader isr = new 
InputStreamReader(in)) {
--- End diff --

Nit: Since you're not using the in variable, you could builder the isr in 
one step


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170418529
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
--- End diff --

Thanks, hadn't thought of that. I was thinking of the case where the full 
path is "*", which I think you've fixed now.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170359655
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1559,8 +1577,8 @@ public static boolean isLocalhostAddress(String 
address) {
 Set ids = srcMap.keySet();
 Integer largestId = ids.stream().max(Integer::compareTo).get();
 int end = largestId - start;
-ArrayList result = new ArrayList<>(Collections.nCopies(end+1 , 
null)); // creates array[largestId+1] filled with nulls
-for( Map.Entry entry : srcMap.entrySet() ) {
+ArrayList result = new ArrayList<>(Collections.nCopies(end + 1, 
null)); // creates array[largestId+1] filled with nulls
--- End diff --

Auto-corrected via IntelliJ. Let me revert if we don't want to fix 
unrelated thing.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170358961
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1574,49 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader invoke() throws IOException {
--- End diff --

The class is made from IntelliJ's extract feature. We are now calling 
readZip/readJar separately so hopefully that is OK to keep it public and be 
called from outside.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170358225
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1574,49 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader invoke() throws IOException {
+try (JarFile jarFile = new JarFile(f)) {
+Enumeration jarEnums = jarFile.entries();
+while (jarEnums.hasMoreElements()) {
+JarEntry entry = jarEnums.nextElement();
+if (!entry.isDirectory()) {
+if (defaultsConf == null && 
entry.getName().equals("defaults.yaml")) {
+try (InputStream in = 
jarFile.getInputStream(entry)) {
+defaultsConf = (Map) 
yaml.load(new InputStreamReader(in));
+}
+}
+
+if (stormConf == null && 
entry.getName().equals("storm.yaml")) {
+try (InputStream in = 
jarFile.getInputStream(entry)) {
+stormConf = (Map) 
yaml.load(new InputStreamReader(in));
--- End diff --

Addressed.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170358164
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
+File[] jarFiles = dir.listFiles((dir1, name) -> 
name.endsWith(".jar"));
+
+if (jarFiles != null) {
--- End diff --

Hmm... yes. Are we better to throw some exception when `dir.listFiles()` 
returns null? Or do we just ignore weird case and get rid of null check?


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170357618
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
--- End diff --

I just get rid of manipulating string via File's method. Please take a look 
at new commit.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

Github user HeartSaVioR commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170357327
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
--- End diff --

According to my understanding of Oracle Java classpath doc, it should only 
expand when file name is '*' rather than like 'abc*'. If 'abc*' is given in 
file name, it should not expand to abcde.jar or so.

https://docs.oracle.com/javase/8/docs/technotes/tools/unix/classpath.html


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170298520
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
--- End diff --

Wouldn't it also work to just check if the part ends with "*"? 


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170300322
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
+File[] jarFiles = dir.listFiles((dir1, name) -> 
name.endsWith(".jar"));
+
+if (jarFiles != null) {
--- End diff --

Nit: Isn't there something really weird going on if this is null? The 
javadoc only mentions that this can be null if there's an IO error or the dir 
isn't a directory.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170301223
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1574,49 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader invoke() throws IOException {
--- End diff --

Nit: This method seems to be part of initializing this class. Consider 
making it private and calling it from the constructor.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170299702
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1505,35 +1508,36 @@ public static StormTopology 
addVersions(StormTopology topology) {
 Map defaultsConf = null;
 Map stormConf = null;
 for (String part: cp) {
-File f = new File(part);
-if (f.isDirectory()) {
-if (defaultsConf == null) {
-defaultsConf = readConfIgnoreNotFound(yaml, new 
File(f, "defaults.yaml"));
-}
-
-if (stormConf == null) {
-stormConf = readConfIgnoreNotFound(yaml, new File(f, 
"storm.yaml"));
+String wildcardSuffix = File.separator + "*";
+if (part.endsWith(wildcardSuffix)) {
+// wildcard is given
+// in java classpath, '*' is translated to '*.jar'
+File dir = new File(part.substring(0, part.length() - 
wildcardSuffix.length()));
--- End diff --

Could we use Path for creating paths instead of manipulating strings? 
(asking because it's nice if the code is obviously portable)


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[srdo]

Github user srdo commented on a diff in the pull request:

https://github.com/apache/storm/pull/2566#discussion_r170301964
  
--- Diff: storm-client/src/jvm/org/apache/storm/utils/Utils.java ---
@@ -1570,4 +1574,49 @@ public static boolean isLocalhostAddress(String 
address) {
 }
 return result;
 }
+
+private static class JarConfigReader {
+private Yaml yaml;
+private Map defaultsConf;
+private Map stormConf;
+private File f;
+
+public JarConfigReader(Yaml yaml, Map 
defaultsConf, Map stormConf, File f) {
+this.yaml = yaml;
+this.defaultsConf = defaultsConf;
+this.stormConf = stormConf;
+this.f = f;
+}
+
+public Map getDefaultsConf() {
+return defaultsConf;
+}
+
+public Map getStormConf() {
+return stormConf;
+}
+
+public JarConfigReader invoke() throws IOException {
+try (JarFile jarFile = new JarFile(f)) {
+Enumeration jarEnums = jarFile.entries();
+while (jarEnums.hasMoreElements()) {
+JarEntry entry = jarEnums.nextElement();
+if (!entry.isDirectory()) {
+if (defaultsConf == null && 
entry.getName().equals("defaults.yaml")) {
+try (InputStream in = 
jarFile.getInputStream(entry)) {
+defaultsConf = (Map) 
yaml.load(new InputStreamReader(in));
+}
+}
+
+if (stormConf == null && 
entry.getName().equals("storm.yaml")) {
+try (InputStream in = 
jarFile.getInputStream(entry)) {
+stormConf = (Map) 
yaml.load(new InputStreamReader(in));
--- End diff --

Nit: Consider moving the InputStreamReader into the try-with-resources, so 
it also gets closed.


---

[GitHub] storm pull request #2566: STORM-2965 Interpret wildcard in classpath correct...

[HeartSaVioR]

GitHub user HeartSaVioR opened a pull request:

https://github.com/apache/storm/pull/2566

STORM-2965 Interpret wildcard in classpath correctly when reading con…

…fig from classpath

* also fix version pattern in SimpleVersion which parses version like 
0.10.3 correctly

Manually tested with running Storm 1.2.1-rc1 topology.

@revans2 Please take a look. Thanks in advance.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/HeartSaVioR/storm STORM-2965

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/storm/pull/2566.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2566


commit 2ea751da4c25024ef874228c82cb0ad747fa0285
Author: Jungtaek Lim 
Date:   2018-02-20T13:07:08Z

STORM-2965 Interpret wildcard in classpath correctly when reading config 
from classpath

* also fix version pattern in SimpleVersion which parses version like 
0.10.3 correctly




---