On Mon, Feb 27, 2017 at 08:39:39PM -0600, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgold...@suse.com> > > This adds JSON support for tools in order to be able to talk to > other utilities such as Yast. > > The json is one per line, in order to differentiate between multiple > records. This is based on work presented by Christian Boltz some time > back. > > Signed-off-by: Goldwyn Rodrigues <rgold...@suse.com>
Hi Goldwyn, thanks for the contribution, I have some comments inline:
> --- a/utils/aa-genprof
> +++ b/utils/aa-genprof
> @@ -61,8 +61,12 @@ parser = argparse.ArgumentParser(description=_('Generate
> profile for the given p
> parser.add_argument('-d', '--dir', type=str, help=_('path to profiles'))
> parser.add_argument('-f', '--file', type=str, help=_('path to logfile'))
> parser.add_argument('program', type=str, help=_('name of program to
> profile'))
> +parser.add_argument('-j', '--json', action="store_true", help=_('provide
> output in json format'))
> args = parser.parse_args()
>
> +if args.json:
> + aaui.UI_mode = 'json'
> +
This block is missing the 'else .. text' that is repeated elsewhere, is
this an oversight?
> profiling = args.program
> profiledir = args.dir
>
> + elif UI_mode == 'json':
> + jsonout = {'info': text}
> + sys.stdout.write(json.dumps(jsonout, sort_keys=False,
> separators=(',', ': ')) + '\n')
I strongly recommend making this a function. Not only is this repeated
often, but I'm pretty sure that Python won't use line-buffered output
in the common case, so each of these should probably have an explicit
sys.stdout.flush() after each one to ensure that Python sends the block
to YaST.
> + #yastLog(text)
.. and commenting out code is one of my pet peeves :) please just delete
it instead.
> @@ -160,14 +166,13 @@ def UI_YesNoCancel(text, default):
> default = 'c'
> else:
> ans = default
> - else:
> - SendDataToYast({'type': 'dialog-yesnocancel',
> - 'question': text
> - })
> - ypath, yarg = GetDataFromYast()
> - ans = yarg['answer']
> - if not ans:
> - ans = default
> + elif UI_mode == 'json':
> + jsonout = {'dialog': 'yesnocancal', 'text': text, 'default': default}
Could you double-check the spelling on this 'yesnocancal' token? (/me
inserts usual grumblings about hash-based programming preventing static
analysis.)
> + sys.stdout.write(json.dumps(jsonout, sort_keys=False,
> separators=(',', ': ')) + '\n')
> + ans = 'XXXINVALIDXXX'
> + while ans not in ['y', 'n', 'c']:
> + ans = getkey()
Is getkey() the right thing to call after sending json down the pipe? Will
de_DE.UTF-8 return 'j' or 'y'? How about ja_JA? (は vs い?)
> +
> return ans
>
> def UI_GetString(text, default):
> @@ -181,44 +186,34 @@ def UI_GetString(text, default):
> string = ''
> finally:
> readline.set_startup_hook()
> - else:
> - SendDataToYast({'type': 'dialog-getstring',
> - 'label': text,
> - 'default': default
> - })
> - ypath, yarg = GetDataFromYast()
> - string = yarg['string']
> + elif UI_mode == 'json':
> + jsonout = {'dialog': 'getstring', 'text': text, 'default': default}
> + sys.stdout.write(json.dumps(jsonout, sort_keys=False,
> separators=(',', ': ')) + '\n')
> +
> + string = raw_input('\n' + text)
> +
> return string.strip()
>
> def UI_GetFile(file):
> - debug_logger.debug('UI_GetFile: %s' % UI_mode)
> + debug_logger.debug('UI_Mode: %s' % UI_mode)
> filename = None
> if UI_mode == 'text':
> sys.stdout.write(file['description'] + '\n')
> filename = sys.stdin.read()
> - else:
> - file['type'] = 'dialog-getfile'
> - SendDataToYast(file)
> - ypath, yarg = GetDataFromYast()
> - if yarg['answer'] == 'okay':
> - filename = yarg['filename']
> + elif UI_mode == 'json':
> + jsonout = {'dialog': 'getfile', 'text': file['description']}
> + sys.stdout.write(json.dumps(jsonout, sort_keys=False,
> separators=(',', ': ')) + '\n')
> + filename = raw_input('\n')
Is raw_input() the right function to run via the json ui?
> + #if UI_mode == 'text':
> + UI_Info(message)
Another instance of commented-out-code, please remove.
>
> def UI_BusyStop():
> debug_logger.debug('UI_BusyStop: %s' % UI_mode)
> - if UI_mode != 'text':
> - SendDataToYast({'type': 'dialog-busy-stop'})
> - ypath, yarg = GetDataFromYast()
>
> CMDS = {'CMD_ALLOW': _('(A)llow'),
> 'CMD_OTHER': _('(M)ore'),
> @@ -300,7 +295,7 @@ class PromptQuestion(object):
> def promptUser(self, params=''):
> cmd = None
> arg = None
> - if UI_mode == 'text':
> + if UI_mode == 'text' or UI_mode == 'json':
> cmd, arg = self.Text_PromptUser()
> else:
> self.type = 'wizard'
> @@ -377,6 +372,15 @@ class PromptQuestion(object):
> function_regexp += ')$'
>
> ans = 'XXXINVALIDXXX'
> + hdict = dict()
> + jsonprompt = {
> + 'title': title,
> + 'headers': hdict,
> + 'explanation': explanation,
> + 'options': options,
> + 'menu_items': menu_items,
> + }
> +
> while not re.search(function_regexp, ans, flags=re.IGNORECASE):
>
> prompt = '\n'
> @@ -388,6 +392,7 @@ class PromptQuestion(object):
> while header_copy:
> header = header_copy.pop(0)
> value = header_copy.pop(0)
> + hdict[header] = value
> prompt += formatstr % (header + ':', value)
> prompt += '\n'
>
> @@ -405,7 +410,10 @@ class PromptQuestion(object):
>
> prompt += ' / '.join(menu_items)
>
> - sys.stdout.write(prompt + '\n')
> + if UI_mode == 'json':
> + sys.stdout.write(json.dumps(jsonprompt, sort_keys=False,
> separators=(',', ': ')) + '\n')
> + elif UI_mode == 'text':
> + sys.stdout.write(prompt + '\n')
>
> ans = getkey().lower()
And, again, a question if getkey() is the right UI mechanism for the json
interface.
Thanks
signature.asc
Description: PGP signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
