[Freeipa-devel] [PATCH 0016] Add RADIUS proxy support to ipalib CLI

Petr Viktorin pviktori at redhat.com
Thu Nov 28 11:18:34 UTC 2013


Thanks!
Just a bit of cleaning up now, sending a patch with proposed changes to 
speed things up.

Patch needs a tiny rebase.
Points I missed:
- There are some unused imports.
- ValidationError takes the attribute name in `name` rather than the 
name of the CLI option.

>> Now the validation is too strict, a port is not accepted.
>
> Fixed.

"invalid!" is pretty bad for an error message. I put it in as a 
placeholder, but I wasn't clear about that, sorry!

>> Should non-FQDN hostnames be allowed?
>
> I agree they should not. Fixed.

validate_hostname() has a check_fqdn argument, no need to do this manually.

>>>> ipatokenusermapattribute is also not validated. Not sure if it needs to be.
>>>
>>> I don't think validation is really possible outside of the permitted
>>> characters for an LDAP attribute.
>>
>> I think if "$%^&*" is allowed, we'll get a bug from QA soon enough.
>
> Fixed.

The `sre` module is named `re` since Python 2.5.

>> We generally output lists; this should also be a list with one element.
>
> Fixed.
>
>> Attaching updated tests.
>
> A few of these tests are still failing for me, but it is not immediately
> obvious why. They seem to be getting answers from previous queries. I'm
> not sure if this is something wrong with my code or the tests. Can you
> take a look at it?

My bad, I've used a wrong variable name.

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0016-7pviktori1-Add-RADIUS-proxy-support-to-ipalib-CLI.patch
Type: text/x-patch
Size: 33017 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131128/4e38e0c2/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0318.3-Add-tests-for-the-radiusproxy-plugin.patch
Type: text/x-patch
Size: 14829 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131128/4e38e0c2/attachment-0001.bin>


More information about the Freeipa-devel mailing list