On 25.9.2014 10:31, Martin Basti wrote:
On 24/09/14 16:24, Martin Basti wrote:
On 24/09/14 16:05, Martin Basti wrote:
On 23/09/14 17:45, Petr Vobornik wrote:
On 25.8.2014 14:52, Martin Basti wrote:
Patches attached.

Ticket: https://fedorahosted.org/freeipa/ticket/4149

There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the
named service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138



Review of:
http://www.redhat.com/archives/freeipa-devel/2014-September/msg00484.html

1. Please follow pep8 for the new code.
 # git diff HEAD~7 -U0 | pep8 --diff --ignore=E501
Produces 25 erros.

Only E124 and E128 could be ignored if they are part of old code.

I left there some pep8 errors. They don't decrease readability


Patch 120:

3. This patch uses term 'deprecated' in a different meaning than a
DeprecatedParam. It creates inconsistency -> future confusion. IMHO this
usage is correct since the usual understanding of deprecation is that the
param is still usable but user should be prepared that it will be removed
in a future.  IMHO DeprecatedParam is badly designed but that's not an
issue of this patch.

I think we can leave this as is and create a ticket to rename
DeprecatedParam e.g. to RemovedParam. What do you think?

https://fedorahosted.org/freeipa/ticket/4566
5. You've removed 'idnssoamname' and 'force' from Web UI but dnszone-add
precallback still uses these params. What is the intended purpose?
User should use modify dialog in webUI for zones.
Precallback fills default value for idnsmname from LDAP.
with --force there will be no validation of user specified soa mname

Purpose is a user should let IPA to fill mname with safe value.
Patch 123:

10. In `normalize_zonemgr(zonemgr)`, if zonemgr contains '@' shouldn't it
be normalized to contain '.' at the end? Or is it handled by bind-dyndb-ldap?

Zone manager (SOA RNAME) can eb relative name, BIND will append zone name.
Currently we cant validate if email address is reachable, it doestn matter
if it is filled with nonexistent absolute name, or nonexistent relative name.

Unrelated to this patch set:

a. One is able to run:
  # ipa dnszone-remove-permission $zone
multiple times and it always returns success

Is it intentional?
No, it isn't. I will inspect it and I will send additional patch


b. Web UI doesn't have means to call dnszone-mod with --force option
I'm not sure what you mean, it didn't do that before my patches.

Updated patches attached

I accidentally removed one line in previous patchset.
Updated patches attached

Sorry my IDE was too smart, and somehow added its configuration file to commit
and I didn't notice it.
Patches attached.

ACK, it works for me. Replica installation and deletion properly adds and deletes records as necessary.

I would defer further improvements to
https://fedorahosted.org/freeipa/ticket/3343

--
Petr^2 Spacek

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

Reply via email to