[Freeipa-devel] [PATCH] 151, 152 Removal of illegal options in association dialog

[Endi Sukma Dewata]

On 6/6/2012 7:12 AM, Petr Vobornik wrote:
> On 06/05/2012 08:38 PM, Endi Sukma Dewata wrote:
>> If I understood correctly the json_exclude_attrs already defines the
>> list of attributes to be excluded, so is it still necessary to define
>> json_only_presence_options which basically will remove all attributes
>> except name? Suppose later you're writing the UI console where you can
>> type the CLI commands in the UI, do you think attributes like doc would
>> be needed to show in the command help?
>
> Depends on the implementation :). I think doc, labels and such would be
> very useful. In such case the missing attributes might be a problem. But
> it can be overcame by supplying it in other way because of the nature of
> the options in only_presences list (their attrs are same everywhere).
>
> Ideally I would not use the json_only_presence_options but it saves
> quite a lot of data.
>
> Btw, regarding the response size: if indentation in json.dump is
> disabled it save about 3/4 of the size - very handy. I don't want to do
> it yet because the ouput is not very readable (for devels). I think
> enabling gzip compression for /ipa/session/json might be a way too.
>
>> If this is fine then ACK on both.
>
> I'm not sure if my reasoning can be considered 'fine'.

If you already have a plan how to handle it later then it's fine. The 
code works as is, so I don't have any objection to push it now.

About the indentation, how about adding a parameter to compress the 
output? So the UI can add something like 'compress=true' when pulling 
the metadata, then for now the server can just remove the indentations. 
Later you can add gzip compression too. For development you can still 
open the URL without this parameter and you'll get a nicely formatted 
output.

>> Btw, the static test data (i.e. ipa_init_*.json) should be updated as
>> well. You might want to create some scripts in install/ui/test/bin to
>> update these files similar to update_ipa_init.sh.
>
> Ah, I forget to update it. I have a script which can handle both. I'll
> include updated ipa_init_object.json and ipa_init_command.json when
> pushing #152.
>
> I'm attaching the script, if you are interested. It works with
> form-based auth, so I can query test machine from devel machine without
> test's krb5.conf. I'm using the script for generating other .json files.
> I haven't put to git because of my frequent personal changes which might
> not be useful for others.

Looks good. If you want you can also put it in a personal git repo, but 
just make sure they don't contain private information. Or make it 
configurable, and you can use a private wrapper script to supply your 
environment-specific settings. I'm doing something similar for PKI:
http://fedorapeople.org/gitweb?p=edewata/public_git/pki-dev.git;a=tree

This is a lower priority, but it might be useful to document how to 
setup a server (including the data) that can be used to generate the 
static test data. It can be step-by-step instructions or scripts.

-- 
Endi S. Dewata