STORM-188: Allow user to specifiy full configuration path when running storm command #120
Conversation
… system path if we cannot find it on classpath
| import java.io.FileOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.InputStreamReader; | ||
| import java.io.ObjectInputStream; | ||
| import java.io.ObjectOutputStream; | ||
| import java.io.PrintStream; |
|
Changed my comments to the diff instead of the commit. Sorry about that. |
|
@clockfly Any chance to look at the comments? |
|
@clockfly Could you upmerge also? |
|
Sure, I will take care of it. |
|
Hi @clockfly, any update? |
…rch file system path if we cannot find it on classpath" This reverts commit 0512fb4.
|
@d2r , sorry it takes so long. Now, the patch is synced with upstream. |
|
The original behavior of findAndReadConfigFile() is to locate config file on classpath. findResources(name) will not be empty when name exists on classpath. |
| input.close(); | ||
| in = getConfigFileInputStream(name); | ||
| if (null != in) { | ||
| Yaml yaml = new Yaml(); |
There was a problem hiding this comment.
Can we change this to Yaml yaml = new Yaml(new SafeConstructor());
|
I just had one comment, to avoid YAML doing some potentially unsafe things, but it is not that critical, because placing a --config on the command line is going to use a file the user probably supplied. |
| + " resources. You're probably bundling the Storm jars with your topology jar. " | ||
| + resources); | ||
| } else { | ||
| URL resource = resources.iterator().next(); |
There was a problem hiding this comment.
It might be nice to log something here, so that the user knows that we are using a config file from the classpath. I just thought about the case where someone creates a ./storm.yaml file and passes it in with --config. They will get the storm.yaml from the classpath without knowing it, because the classpath is searched prior to the absolute path being searched, and this would be really confusing.
|
@clockfly any update on this? I just had the one minor comment, and I am happy to do it myself in a follow up JIRA if you want. Just curious if this is still on your radar. |
|
Feel free to do what you want, change it, or replace it.:) |
Derekd logdeletion permission
|
guys, any updates on this issue ? |
|
Sure. |
We can use system config "storm.conf.file" to specify a custom config file.
Before this patch, we will only try to lookup this file on jvm classpath.
This patch will give us the ability to specify full/relative filesystem path as configuration file path, thus will give us more flexibility, while we still are able to look it up on jvm classpath.
Now we are able to submit storm topology like this:
We don't need to add /tmp into our classpath(also inappropriate).