[ZIv6F2QY] Allow Command Expansion APOC #453
Conversation
| "%s is a command, but config is not explicitly told to expand it. (Missing --expand-commands argument?)", | ||
| entry)); | ||
| String str = entry.trim(); | ||
| String command = str.substring(2, str.length() - 1); |
There was a problem hiding this comment.
Does neo4j only admit command expansion with $(command) or is there other way? I'm asking because in bash I think you can also enclose in backticks:
These two produce the same result:
echo hola $(echo 'adios')
echo hola `echo 'adios'`
There was a problem hiding this comment.
Yeah, so according to the docs:
The scripts are specified in the neo4j.conf file with a $ prefix and the script to execute within brackets (), i.e., dbms.setting=$(script_to_execute).
Also the code defines a command as:
private static boolean isCommand(String entry) {
String str = entry.trim();
return str.length() > 3 && str.charAt(0) == '$' && str.charAt(1) == '(' && str.charAt(str.length() - 1) == ')';
}
So I guess APOC should follow Neo here, even if bash allows back spaces :)
| addLineToApocConfig(invalidExpandLine); | ||
|
|
||
| RuntimeException e = assertThrows(RuntimeException.class, apocConfig::init); | ||
| String expectedMessage = "java.io.IOException: Cannot run program \"fakeCommand\": error=2, No such file or directory"; |
There was a problem hiding this comment.
Could we wrap this in a better exception so someone knows what config option failed to expand exactly?
There was a problem hiding this comment.
So this exception would be written in the logs on startup, which causes Neo to not start, and directly before the line:
log.info("Executing external script to retrieve value of setting " + settingName);
Gets printed, so the user should see that and the exception. Is that what you were thinking? I can also catch the exception and add more info, but then it will behave differently to Neo (this error comes from Neo)
There was a problem hiding this comment.
Yep, the current log is good enough then in my opinion
gem-neo4j
force-pushed
the
dev_allow_command_expansion_apocConf
branch
2 times, most recently
from
09f426b
to
64ece1a
Compare
gem-neo4j
force-pushed
the
dev_allow_command_expansion_apocConf
branch
from
a54559a
to
82a5f38
Compare
| if (this.commandEvaluationTimeout == null) { | ||
| this.commandEvaluationTimeout = GraphDatabaseInternalSettings.config_command_evaluation_timeout.defaultValue(); | ||
| } | ||
| this.expandCommands = neo4jConfig.expandCommands(); |
There was a problem hiding this comment.
At this point, do we already know that config for expanding is set? Probably we shouldn't even try if the option is off.
There was a problem hiding this comment.
I'm not sure I follow, expand command can only be set on startup, so it would already be set as Neo4j starts APOC with the Neo4J config 🤔 Which part shouldn't we try?
There was a problem hiding this comment.
Sorry, my misunderstanding of the code, ignore this comment.
Depends on Neo4j PR for reading expandCommands.
Follows what Neo4j has: