Rob Crittenden wrote:
Pavel Zuna wrote:
Rob Crittenden wrote:
Pavel Zuna wrote:
I compiled 3 patches, that effectively bring back all the
functionality we had before Jasons big patch (i.e. before
introducing output validation and the common output interface).
--all and --raw are back, but this time as global options
replacing DNs with primary keys is back
clever attribute printing (word-wrapping etc.) is back too
To implement --all and --raw as global options, we had to find a way
to propagate additional information (apart from command name and
parameters) from client to server. We extended the XML-RPC signature
from:
(arg0, arg1, ..., options)
to:
(args, options, extras)
The extras dict is currently only filled with the 'print_all_attrs'
and 'print_raw_attrs' settings when forwarding a call. The server
saves the extras dict into the thread specific context variable.
I also replaced the decoding table in Encoder, because it didn't
really work as expected in special cases. It now uses a dont-decode
function. In the case of ldap2, this function checks attribute type
OIDs and returns False for binary types.
This patch introduces a little problem with the env command, because
it fixes a bug/feature, that made it work before. Before outputting
an attribute, we check if it isn't of type str. If it is, we assume
it is binary and decode it. All values in Env are str. I propose we
either write a specific output_for_cli for the env command or think
about switching from str to unicode. I tried the later and it didn't
cause any problems so far.
How it's supposed to work:
# ./ipa user-show admin
User login: admin
Last name: Administrator
Home directory: /home/admin
Login shell: /bin/bash
# ./ipa --all user-show admin
dn: uid=admin,cn=users,cn=accounts,dc=pzuna
User login: admin
Last name: Administrator
Full name: Administrator
Home directory: /home/admin
GECOS field: Administrator
Login shell: /bin/bash
Kerberos principal: ad...@pzuna
UID: 1083719807
GID: 1083719807
Last password change date: 20100208132706Z
Password expiration date: 20100509132706Z
Member of groups: admins
objectclass: top, person, posixaccount, krbprincipalaux,
krbticketpolicyaux, inetuser
# ./ipa --raw user-show admin
uid: admin
sn: Administrator
homedirectory: /home/admin
loginshell: /bin/bash
# ./ipa --all --raw user-show admin
dn: uid=admin,cn=users,cn=accounts,dc=pzuna
uid: admin
sn: Administrator
cn: Administrator
homedirectory: /home/admin
gecos: Administrator
loginshell: /bin/bash
krbprincipalname: ad...@pzuna
uidnumber: 1083719807
gidnumber: 1083719807
krblastpwdchange: 20100208132706Z
krbpasswordexpiration: 20100509132706Z
memberof: cn=admins,cn=groups,cn=accounts,dc=pzuna
objectclass: top
objectclass: person
objectclass: posixaccount
objectclass: krbprincipalaux
objectclass: krbticketpolicyaux
objectclass: inetuser
Pavel
Generally looks ok, have some questions though:
- We currently rely on the fact that binary objects are encoded as
python str, it's how we determine what to base64-encode. What
mechanism will we have to do that now?
I didn't (and I'm not planning to) make any changes in this matter.
My point is that for binary objects we were explicitly setting their
type to str. We don't seem to be doing that any more, so are we relying
on python-ldap to default to the str type? It's ok if we do I'd just
like to see a comment to that effect in case something changes in the
future.
Yeah, we do rely on python-ldap in this case. It returns everything as
str. I didn't realize you were referring to the changes in the Encoder
class.
Some background information about Encoder:
When I started working on the ldap2 backend, I realized that around
every call to python-ldap, we had to encode/decode both compound and
scalar values. With scalar values, it wasn't a problem to just choose
what to encode/decode and what not. With compound values likes entries,
it was more difficult, because all attributes are returned as str, but
have different types. I implemented a feature in the Encoder class, that
enabled its consumers to define a decoding table for dicts and a
function of the dict key, that would return a key in the decoding table.
The decoding table was supposed to contain callables (mostly python
types), that would be used to decode the dict value. If the returned key
was not in the table, default decoding (to unicode) would take place.
The idea was, that we would convert boolean strings to bool, integer
values to int and leave binary values as str. Unfortunatly, there were
some difficulties with boolean types, then Simo chimed in about integers
in LDAP not having the same range as int in python and you can't argue
with Simo. Plus I didn't really feel like doing anything about the
booleans, so the decoding table in ldap2 was just used to leave binary
attributes as str. With the latest Encoder patch, I decided to remove
the decoding table completely and replace it with a dont-decode function
of the dict key, because that's just what we need in ldap2 and the
decoding table wasn't working properly with complex compound types
anyway (e.g. dicts in a dict).
What I'm saying is that the Env object stores all strings as str and
the env command uses the same output_for_cli as LDAP commands, that
only use str for binary. So, we either need to override output_for_cli
or switch to unicode in Env.
Not exactly sure what to do here though using unicode seems like the
best route.
- Is print_* the right prefix for these new global variables? It
affects more than just printing in the case of all because it returns
everything over XML-RPC as well.
You're right, maybe get_* or something similar would be better. I'm
open to suggestions.
I'm ok with print_raw because that is what it does. maybe print_all ->
retrieve_all?
- Is there/should there be a way for a plugin to define its own
extras? And not to be too pedantic but is extras the best description
for these values? Not that I have any suggestions for an improvement
:-( Perhaps global_options?
The extras dict is there to pass additional information, that is
command independent. Commands probably shouldn't define their own. I
say probably, because it is possible, that we're going to find out
this is actually the best way to accomplish something.
Extras might not be the best description, but we need something
general, because it can contain pretty much anything and not just
global options.
Ok, I don't want to agonize too much over a variable name.
- Why are you removing get_options() from LDAPSearch()?
Because it was only used to generate an option for the UUID attribute.
Since Jason's no_create,no_update patch it isn't needed anymore,
because we can just define an UUID param with these flags set.
Ok.
It doesn't look like this is going to conflict too much with the
parallel work I've done in regard to including member/memberof in
return values, nor in the output work I've done. So you don't need to
work on the individual plugins at all, I've got that ready in my tree
though I'm going to hold onto it until we can get these patches
committed.
Cool, that's good to hear... er, I mean read. :) I was a bit worried,
because on the meeting you sounded like you spent quite a bit of time
working on it and having to deal with conflicts/merges/andwhatnot
because of my work wouldn't be the most appropriate reward. :)
Yeah, minus your 4th patch the merge went incredibly smoothly. My patch
covered most of what it did with a few exceptions on some default
parameters.
Great. :)
rob
Pavel
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel