modules/adblockplus/templates/mimeo.service.erb - Issue 29504594: #2687 - Include mimeo python module

Delta Between Two Patch Sets: modules/adblockplus/templates/mimeo.service.erb

Issue 29504594: #2687 - Include mimeo python module (Closed)

Left Patch Set: For comments 28 to 30 Created Aug. 16, 2017, 4:14 p.m.

Right Patch Set: For comments 47 and 48 Created Aug. 22, 2017, 8:33 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 [Unit]	1 [Unit]

2 Description=Service that handles formating and parsing of logs	2 Description=Registers infromation received by an http/s petition.

3	3

4 [Service]	4 [Service]

5 ExecStart=/usr/local/bin/mimeo.py '--response=<%= @response.to_s.to_json %>' '-- format=<%= @format %>' '--port=<%= @port%>' <%= @log_file %>	5 ExecStart=/usr/local/bin/mimeo.py '--response=<%= @response %>' '--format=<%= @f ormat %>' '--port=<%= @port %>' /var/adblockplus/mimeo/data
mathias 2017/08/18 08:47:47 This is not what we discussed and a bad copy of my This is not what we discussed and a bad copy of my suggestion. Instead of `--response <%= @reponse.to_s.to_json %>` (two separate parameters where the 2nd takes advantage of JSON escaping and quotation) you now use a construct that results in the output message not being `response` but `"response"` with quotes (or am I reading that wrong?). The rationale applies to all parameters, hence it should be `--format <%= @format.to_s.to_json %>` and `<%= @log_file.to_s.to_json %>` (how about renaming that one to also not use `log_`, btw?). The `--port=<% @port.to_i %>` could be given as one, though for the sake of consistency it should be two separate arguments as well - IF you agree with my suggestions. f.lopez 2017/08/19 00:51:02 Well what you suggested is documented and it's sup Show quoted text On 2017/08/18 08:47:47, mathias wrote: > This is not what we discussed and a bad copy of my suggestion. Instead of > `--response <%= @reponse.to_s.to_json %>` (two separate parameters where the 2nd > takes advantage of JSON escaping and quotation) you now use a construct that > results in the output message not being `response` but `"response"` with quotes > (or am I reading that wrong?). > > The rationale applies to all parameters, hence it should be `--format <%= > @format.to_s.to_json %>` and `<%= @log_file.to_s.to_json %>` (how about renaming > that one to also not use `log_`, btw?). The `--port=<% @port.to_i %>` could be > given as one, though for the sake of consistency it should be two separate > arguments as well - IF you agree with my suggestions. Well what you suggested is documented and it's supposed to work, but I tried different ways and I couldn't make it work with the C-style escape. The documentation says "C-style escapes are also supported" so I just replaced the backquotes for normal single quotes, also the docs says "the opening quote may appear only after whitespace, and the closing quote must be followed by whitespace or the end of line" which made me use the "=" (equal) character in order to avoid whitespaces in between, which resulted in this version. Am I missing something? I tried several options using different escape mechanisms and this one is the one I made it work every time reliable. mathias 2017/08/19 15:06:08 I have no idea what you mean. I did not mention an Show quoted text On 2017/08/19 00:51:02, f.lopez wrote: > On 2017/08/18 08:47:47, mathias wrote: > > This is not what we discussed and a bad copy of my suggestion. Instead of > > `--response <%= @reponse.to_s.to_json %>` (two separate parameters where the > 2nd > > takes advantage of JSON escaping and quotation) you now use a construct that > > results in the output message not being `response` but `"response"` with > quotes > > (or am I reading that wrong?). > > > > The rationale applies to all parameters, hence it should be `--format <%= > > @format.to_s.to_json %>` and `<%= @log_file.to_s.to_json %>` (how about > renaming > > that one to also not use `log_`, btw?). The `--port=<% @port.to_i %>` could be > > given as one, though for the sake of consistency it should be two separate > > arguments as well - IF you agree with my suggestions. > > Well what you suggested is documented and it's supposed to work, but I tried > different ways and I couldn't make it work with the C-style escape. > The documentation says "C-style escapes are also supported" so I just replaced > the backquotes for normal single quotes, also the docs says "the opening quote > may appear only after whitespace, and the closing quote must be followed by > whitespace or the end of line" which made me use the "=" (equal) character in > order to avoid whitespaces in between, which resulted in this version. > > Am I missing something? I tried several options using different escape > mechanisms and this one is the one I made it work every time reliable. I have no idea what you mean. I did not mention anything C-style. Try reading my comment again, but note that a) BACKTICKS (`) enclose code-snippets in my examples, but aren't part of the actual code, b) I did explicitly refer to two separate parameters and NOT a single one where key and value are separated by an equals sign (=), and c) there is a difference between quoting and escaping, e.g. there's no such thing as a BACKQUOTE.
6	6

7 [Install]	7 [Install]

8 WantedBy=multi-user.target	8 WantedBy=multi-user.target

9	9

LEFT	RIGHT