[Freeipa-devel] [PATCHES] Bring master up to Jason's tree

[Jason Gerard DeRose]

On Mon, 2009-02-02 at 14:35 -0500, Rob Crittenden wrote:
> Rob Crittenden wrote:
> > Once we get things settled in master I'll leave it up to Jason to keep 
> > it up to date :-)
> > 
> > For now, this refreshes from his tree. It is 38 patches. I've already 
> > done a cursory review. I'm going to respond to my own e-mail with some 
> > issues.
> > 
> > I think it would be easiest if we commit all these patches and then 
> > address any issues found. It will make moving forward easier.
> 
> Ok, here are the things I've found
> 
> - Some of the tests where hosts are involved were switched from a 
> hardcoded ipaexample.$DOMAIN to the current host. I purposely used this 
> hostname so we could avoid collisions with real services. The tests 
> actually delete data. Even though this shouldn't be run against a 
> production server...
> 
> - In KerbTransport() I originally was setting extra_headers directly. It 
> was changed at some point to +=. I think this is probably better since 
> extra_headers is passed in as an argument. We don't want to overwrite 
> things passed in.

One thing I encountered here, Rob: I set it to += not realizing that
extra_headers can be None.  So we should check for this and do something
like:

    added = [<blah blah>]
    if extra_headers is None:
        extra_headers = added
    else:
        extra_headers += added

I don't know if the base class will return headers we actually need, but
this is good future-proofing either way.

> - The # of plugins in misc/plugins.py is misleading. It is really the # 
> of functions in the API, not the number of plugins.

But each function (Command) is in fact a plugin, just usually a rather
minimal one.  And `./ipa plugins` shows the plugins loaded in all
namespaces (api.Backend, api.Object, etc.), not just those in
api.Command.

You can also see what plugins are loaded on the server using the
--server option, like this:

    ./ipa plugins --server

The above requires the server (XML-RPC) to be running and reachable.
From the source tree you can also run the CLI script with
in_server=True, which will cause the script to load the available server
plugins and do the execute internally rather than forwarding it.  This
is very helpful in debugging because the command will do the same thing
but without the complexity of the XML-RPC call.  Here is an example:

    ./ipa -e in_server=True plugins  # Same list as above

Or:

    ./ipa -e in_server=True user-add jderose

It will probably be most productive to develop your plugins this way.
You can make this the default by adding it in your ~/.ipa/cli.conf file,
like this:

    # Put this in ~/.ipa/cli.conf
    # (but without the leading spaces in this email)
    [global]
    in_server = True

But you should also test your plugins over XML-RPC.  Even with
in_server=True in your cli.conf, you can still override it on the
command line, like this:

    ./ipa -e in_server=False user-add jderose  # Will forward to server

Those are my random tips for the day.

> - The regisgrations of the ra plugin are currently all commented out
> 
> -In some of the tests I use things like res.get('somevalue','') instead 
> of res['somevalue'] so we don't crash on KeyError. Either way the test 
> will fail, not sure which is easier to understand.

Since the test fails either way, I recommend res['somevalue'] as
res.get() makes it seem like there might be more than one condition
under which the test can pass.  If the 'somevalue' key is missing, the
KeyError raised makes the trace easy to understand.

For cases when the key is present but the value is incorrect, it's
helpful to include the incorrect value in the assert message, like this:

    >>> assert res['somevalue'] == 'The value', repr(res['somevalue'])
    Traceback (most recent call last):
      ...
    AssertionError: 'Not the value'

Otherwise the trace wont show the offending value.  I'll try to be
better about doing this in the unit tests for the core library.

> rob
> 
> _______________________________________________
> 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: signature.asc
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090202/7e66ab88/attachment.sig>