[Freeipa-devel] [PATCH] jderose 027 Extensible return values

Wed Dec 9 16:12:46 UTC 2009

Okay, here's a revised patch.

Significant additions/changes from the previous version are:

1. The return value dict now includes a 'summary' value, something like
'Added user "jdoe"'.  This summary is used by the CLI and webUI.
Previously I was generating the summary in the CLI and webUI separately.
This removes the duplication and allows the commands to easily produce
arbitrary summaries (before they were limited a single summary format
like 'Added user "%(primary_key)s"'.  This also makes it easier for
3rd-party tools to provide UIs without having to introspect the Python
API (because they happen to be written in PHP, whatever).

2. I renamed the 'primary_key' member in the return value dict to
'value'.  This is simpler and will be will be easier on translators
('Added user "%(primary_key)s"' vs 'Added user "%(value)s"').  I'm also
thinking of returning the name of the primary_key (e.g., 'uid') when
returning an entry or a list of entries, so this opens the door for me
to use 'key' in the future without confusion.  Note this change is only
relative to my previous proposed patch.  The use of the return value
dict hasn't yet hit master.

3. XMLRPC_test.setUp() no longer tests for server availability with
`user-show notfound` prior to each test running.  Instead, I try to
connect to the server just once when the `xmlrcp_test` module loads,
which sets the `server_available` module attribute.  XMLRPC_test.setUp()
will still raise nose.SkipTest for each test as before.  This change
helps the XMLRPC tests run much faster and also makes problems easier to
debug server-side as there isn't all the `user-show notfound` background
noise.

4. This adds my new `Declarative` base class for the XMLRPC tests which
allows you to define the XMLRPC tests using simple data structures,
letting the base class do the tedious stuff.  IHMO, the tests are
considerably faster and easier to write this way, but just as important
is the fact that Declarative takes care of reporting the errors when a
command's return value doesn't match what we expected.  We have pretty
good coverage in the XMLRCP tests, but we don't have very good reporting
when something goes wrong.  I've put a lot of effort into making sure
typical error reports contain the information needed to quickly focus in
on the problem.  The most important part of the error reporting is in
the new tests.util.assert_deepequal() function, which can be used by any
test to compare two nested data structures.  Currently only the
test_user_plugin and test_group_plugin tests are using `Declarative`,
but the rest will follow.

5. I rewrote the make-test script in Python and added a feature John
asked for and one I wanted.  John wanted the ability to easily run only
the tests in one or more modules.  You can now be specifying the module
in Python notation or the module file.  For example:

    ./make-test tests.test_xmlrpc.test_user_plugin

Or equivalently:

    ./make-test tests/test_xmlrpc/test_user_plugin.py

I wanted an easy way to use the nosetests --stop option, which causes
the testing to abort upon reaching the first error, which I have found
useful when updating plugins to one of my incompatible API changes.  Use
it like this:

    ./make-test --stop

Yup, big!  May my patch reviewers one day forgive me.

-Jason

On Wed, 2009-11-25 at 08:35 -0700, Jason Gerard DeRose wrote:
> This patch is big.  There are some significant changes, but easily 50%
> of this patch is made up of small changes to plugins and their unit
> tests to port them to the lower-level API changes.  All unit tests pass
> assuming you install without the --ca option.  I didn't touch the
> cert/ra plugins as I know John has pending changes there.
> 
> Here's a summary:
> 
> 1. Our standard return value is now a dict, which gives us a nice way to
> extend the API without breaking clients/code.  So in this respect, the
> return values are more deeply nested.  Basically if you returned 'foo'
> before, now you return {'result': 'foo'}.
> 
> 
> 2. Previously nested data-structures have been flattened.  Most
> importantly, these have been changed:
> 
>   Entry: [dn, entry] => {'result': entry}
>   Entries: [entries, truncated] => {'result': entries, 'truncated': truncated}
> 
> This was mostly done so that an entry/row/object/whatever can be
> consistently represented as a dict... whether it's from LDAP, or a
> database, or whatever else IPA might get glued into.  Likewise, lists
> are now consistently used to represent lists of things, rather than also
> a struct-like pair, triple, etc.
> 
> 
> 3. We now have a way to specify a return value schema, and this schema
> is enforced.  The top-level schema of the return value dict is described
> using the Param-like classes in ipalib/output.py.  For example:
> 
>   from ipalib import output
> 
>   class user_add(LDAPCreate):
> 
>       has_output = (
>           output.Entry('result'),
>           output.Output('primary_key', unicode, 'The user uid')
>       )
> 
> This hypothetical `user_add` expects you to return something like this:
> 
>   dict(
>       result=dict(givenname=u'Foo', sn=u'Bar', uid=u'fbar'),
>       primary_key=u'fbar',
>   )
> 
> A few commands have C clients whose API would break with these changes.
> If your command is one of them, you can disable the return value
> validation for now by setting the 'use_output_validation' attribute to
> False, like this:
> 
>   class join(Command):
>       use_output_validation = False
> 
> I already did this for join so the installer doesn't break.
> 
> The plugin browser now shows the output schema.  Just fire up the
> in-tree server like this:
> 
>   ./lite-server.py
> 
> And then go to http://localhost:8888/ipa/ui/Command
> 
> 
> 4. I'm using the return values themselves to drive CLI and webUI output.
> As I've worked on extending meta-data for the webUI, I've realized a lot
> of it can be delivered in the return values themselves.  This is good
> because we display the output more on what the commands actually return,
> less on what they merely say they will return... this keeps us honest
> and is another way to ensure we live up to our API contracts.
> 
> My goal is to have a single base output_for_cli() implementation, and
> this patch get's us 90% there already.  Not all the commands have been
> updated, but this will continue in additional patches (or be delegated).
> 
> More webUI functionality has been enabled also, but not as much as I
> hoped for as work on the command plugins took so much time.  The
> commands return a page now instead of JSON.
> 
> 
> 5. I stubbed out the remained missing gettext features we need.  They
> aren't hooked up to gettext yet, but for plugin writers, the API is now
> complete.  See ipalib/text.py for the implementation and
> ipalib/plugins/user.py for a good example of how they are used.
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jderose-027.2-Extensible-return-values.patch
Type: text/x-patch
Size: 217565 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20091209/5c838e76/attachment.bin>