[
https://issues.apache.org/activemq/browse/CAMEL-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=58269#action_58269
]
Mitko Kolev commented on CAMEL-2549:
------------------------------------
Hi Martin and Claus,
thanks for the detailed review. I'll try to answer the questions:
>> 1) Why do your write the results from stdout to the message body AND header?
>> Writing it to the body should be sufficient (preferrably as InputStream).
In this (outCapable) case I write it additionally in the header to be
consistent with the (in-only) case. There are 3 possible output values: exit
value, stdout and stderr. In the in-only case both stdout and stderr, as well
as the exit value are set as headers; the in-message is not touched. Now you
know my concerns, what do you think would be most appropriate in this case?
There is something else here: I initially assumed that it would be better to
keep the component simple. Maybe it is better to introduce a new optional
"encoding" parameter, so that the stdout and stderr have the right encoding
(currently the default system codec is used).
>> What if the executable writes the results to a file. How should these be
>> read then, in your opinion?
In this case this would be similar to a call to System.setOut(new
PrintStream(new FileOutputStream("file.output"). What will happens is that the
standart output stream (stdout) will be empty, because nothing will be written
to it. The output is printed namely in the new file stream. In this case one
should define a custom executor and then read the file output manually.
>> What if the executable reads from stdin. How would one pass data in this
>> case?
The scenario is not supported by this initial implementation for simplicity.
This can be be added further if needed.
>> In ExecParseUtils you do a lot of low-level command line tokenizing/parsing.
>> Aren't there libraries available for that?
There is an Apache library for this - commons-exec. The funcitonality of the
low level parser is actually a fix for an open critical bug, regarding quoting
with \"\"quoted text\"\" (double - double quoted text) in commons exec
http://issues.apache.org/jira/browse/EXEC-36, that is not plannedfor fixing.
The bug occurs, when the arguments are given to a shell script (e.g. windows)
in this case the semicolons or colons are treated as argument separators, and
finally the batch script receives wrong args. I discovered this when i wrote my
first test script, so i assume that more advanced uses of the component will
also encounter the same hard-to-find problem.
I plan to contribute the functionality of the low level parser in Apache
commons-exec. When this happens (or when they do a fix) the parser class can be
dropped. You are completely right, Camel is not the best place for such a low
level parser.
>> Why do you define an ExecBinding interface + impl but do parts of
>> reading/writing from/to an Exchange in ExecProducer directly? This should be
>> the responsibility of the ExecBinding impl.
You are right, this should be refactored
>> A minor one: A timeout value should be typed as long instead of int (a hint
>> in the javadocs that timeout is in milliseconds would be helpful too)
Yes, this must be changed to long.
>> I also agree with Claus' comments regarding # notation.
Yes, I also agree. Should be changed.
> Camel Exec component
> --------------------
>
> Key: CAMEL-2549
> URL: https://issues.apache.org/activemq/browse/CAMEL-2549
> Project: Apache Camel
> Issue Type: New Feature
> Affects Versions: 2.3.0
> Environment: Tested on Windows XP and Linux
> Reporter: Mitko Kolev
> Assignee: Martin Krasser
> Fix For: 2.3.0
>
> Attachments: camel-exec-patch.diff, camel-exec-patch2.diff
>
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
