Rob Crittenden wrote:
Jason Gerard DeRose wrote:
On Wed, 2009-11-25 at 12:05 -0500, Rob Crittenden wrote:
This is purely from reading the patch, I haven't applied and tested it yet.

ipalib/output.py:
+primary_key = Output('primary_key', unicode,
+    'The primary key of the deleted entry'
+)

This isn't only for deleted entries, right?

Ah, yeah, that should be made more generic.  This doc message is only
used by developers, though.

This import doesn't seem to be used:
from inspect import getdoc

What is dont_output_for_cli()? Is this an effort to make things work while we're in transition?

Yeah, I just renamed some methods so we can reference how they were
implemented.  Temporary.

You seem to have disabled the raw option in LDAPSearch, was that intentional?

Originally I got the impression we weren't going to keep both --raw and
--all, but this can be changed.

Is cli_name being dropped for label? I'm ok with that but should we remove it from all the plugins?

No, here is how they work:

`cli_name` is used for the optparse names and defaults to Param.name,
like:

  --first

`label` is a human readable, translatable string.  It's used in the
webUI, and to prompt show entries on cli, like:

  First name: John Doe

`doc` is human readable help passed to optparse.make_option(help=doc).
It default to the value of the label.  It's used like this:

  --uid=INT        UID (use this option to set it manually)

In the above case the `label` is "UID" (not shown) but the `doc` is this
longer string.

The user plugins provide good examples of how I think these should be
used.

I'll submit a patch later documented these different string uses.

rob


We'll also need to determine what we'll do about all the plugins. The cert plugin, for example, isn't ported to this new return value system and blows up in many places.

There are also some labels missing, such as for fqdn in the host plugin.

These are both quite easy to fix, I think we just need to coordinate things. Perhaps if Pavel and I split up the plugins and fix anything that needs fixing and commit all the patches at one time to avoid any period of breakage.

rob

Just did a fast forward through the big patch. It looks mostly OK, but as Rob said - it breaks a few things. I don't mind fixing all the plugins - it shouldn't be too hard, because at this point most of them are just extensions of baseldap.py classes. I'm going to apply the patch on my tree and see what I can do in the second half of this week.

One thing I noticed:

+        return dict(
+            result=entry_attrs,
+            primary_key=keys[0],
+        )

This will work on most plugins, but you should use keys[-1], because keys might contain parent object keys as well. The last key is always the primary key of the object in question.

Pavel

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to