On 10/25/2017 05:20 PM, Christian Boltz wrote:
> Hello,
>
> Am Montag, 23. Oktober 2017, 12:38:34 CEST schrieb Goldwyn Rodrigues:
>> From: Goldwyn Rodrigues <rgold...@suse.com>
>>
>> Provides the filename in the json format, which can be
>> directly read by Yast. Increased the protocol version; perhaps
>> it should go in the next release.
>>
>> Signed-off-by: Goldwyn Rodrigues <rgold...@suse.com>
>> ---
>> utils/apparmor/ui.py | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/apparmor/ui.py b/utils/apparmor/ui.py
>> index be07b28a..2afbd5b1 100644
>> --- a/utils/apparmor/ui.py
>> +++ b/utils/apparmor/ui.py
>> @@ -45,7 +45,7 @@ def write_json(jsonout):
>> def set_json_mode():
>> global UI_mode
>> UI_mode = 'json'
>> - jsonout = {'dialog': 'apparmor-json-version', 'data': '2.12'}
>> + jsonout = {'dialog': 'apparmor-json-version', 'data': '2.13'}
>> write_json(jsonout)
Based on today's discussion. Should this version be any different?
We do have a catchall for invalid dialog in yast, so we may not require
it after all.
>>
>> # reads the response on command line for json and verifies the
>> response @@ -250,9 +250,16 @@ def
>> generate_diff_with_comments(oldprofile, newprofile): def
>> UI_Changes(oldprofile, newprofile, comments=False):
>> if comments == False:
>> difftemp = generate_diff(oldprofile, newprofile)
>> + header = 'View Changes'
>> else:
>> difftemp = generate_diff_with_comments(oldprofile, newprofile)
>> - subprocess.call('less %s' % difftemp.name, shell=True)
>> + header = 'View Changes with comments'
>> + if UI_mode == 'json':
>> + jsonout = {'dialog': 'changes', 'header':header, 'filename':
>> difftemp.name} + write_json(jsonout)
>> + response = json_response('changes')["response"]
>
> make check complains:
> apparmor/ui.py:260: local variable 'response' is assigned to but never
> used
>
> Obviously nobody cares about the response of displaying the diff (but
> expecting a response is still a good idea to keep the interface
> consistent). Therefore I'll change this line to
>
> + json_response('changes')["response"] # response gets ignored,
> therefore not assigning to a variable
I won't say it is consistent, because apparmor-json-version does not
require a response.
Yes, thats how it should be. I added a response primarily so that the
temporary file does not get deleted on closure. Perhaps you may want to
add that in the comment.
>
>> + else:
>> + subprocess.call('less %s' % difftemp.name, shell=True)
>> difftemp.close()
>>
>> CMDS = {'CMD_ALLOW': _('(A)llow'),
>
> With the above change:
> Acked-by: Christian Boltz <appar...@cboltz.de>
>
>
> Regards,
>
> Christian Boltz
>
>
>
--
Goldwyn
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
