[apparmor] [PATCH 2/2] Add JSON interface to UI_Changes

[Christian Boltz]

Hello,

Am Donnerstag, 26. Oktober 2017, 02:10:27 CEST schrieb Goldwyn Rodrigues:
> On 10/25/2017 05:20 PM, Christian Boltz wrote:
> > Am Montag, 23. Oktober 2017, 12:38:34 CEST schrieb Goldwyn Rodrigues:
> >> From: Goldwyn Rodrigues <rgoldwyn at 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 <rgoldwyn at 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?

Good question - no released version contains the JSON code, and YaST is 
the only consumer. So...

> We do have a catchall for invalid dialog in yast, so we may not
> require it after all.

... if you are fine with keeping '2.12' as JSON version, I don't see any 
problems with it.

> > 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.

Indeed, that sounds like a better comment ;-)


Here's the follow-up patch - Goldwyn, is that what you had in mind?


[patch] Keep JSON version at 2.12

We never did a release with the JSON code, and YaST (the only known user
of the JSON interface) will work with the added 'changes' dialog type 
from r3721 without needing changes.

Also add a better comment/reason why a response for 'changes' is 
expected, but gets ignored.

=== modified file 'utils/apparmor/ui.py'
--- utils/apparmor/ui.py        2017-10-25 22:36:48 +0000
+++ utils/apparmor/ui.py        2017-10-26 11:42:10 +0000
@@ -45,7 +45,7 @@
 def set_json_mode():
     global UI_mode
     UI_mode = 'json'
-    jsonout = {'dialog': 'apparmor-json-version', 'data': '2.13'}
+    jsonout = {'dialog': 'apparmor-json-version', 'data': '2.12'}
     write_json(jsonout)
 
 # reads the response on command line for json and verifies the response
@@ -257,7 +257,7 @@
     if UI_mode == 'json':
         jsonout = {'dialog': 'changes', 'header':header, 'filename': difftemp.name}
         write_json(jsonout)
-        json_response('changes')["response"]  # response gets ignored, therefore not assigning to a variable
+        json_response('changes')["response"]  # wait for it to delay deletion of difftemp (and ignore response content)
     else:
       subprocess.call('less %s' % difftemp.name, shell=True)
     difftemp.close()


Regards,

Christian Boltz
-- 
> Can I get some more info from the machine? 'dmesg', 'cat
> /proc/bus/input/devices', etc ...
Sorry, there's no command calles "etc" on my machine... ;-)
[Rasmus Plewe on https://bugzilla.novell.com/show_bug.cgi?id=176022]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20171026/a7e2b936/attachment.sig>