Discussion:
[Freeipa-devel] [freeipa PR#681][opened] Fix ipadiscovery
alex-zel
2017-04-02 09:04:44 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Author: alex-zel
Title: #681: Fix ipadiscovery
Action: opened

PR body:
"""
Sort SRV records for LDAP/KRB based on priority.

"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/681/head:pr681
git checkout pr681
tiran
2017-04-03 04:14:16 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Title: #681: Fix ipadiscovery

tiran commented:
"""
You can simplify your code a lot with the operator module and sorted(key) trick:

https://docs.python.org/3/library/operator.html#operator.attrgetter
https://docs.python.org/3/library/functions.html#sorted

```
import operator
```

```
answers = resolver.query(qname, rdatatype.SRV)
answers = sorted(answer, key=operator.attrgetter('priority'))
```

Please squash your changes into one commit.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/681#issuecomment-291045102
martbab
2017-04-03 07:08:56 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Title: #681: Fix ipadiscovery

martbab commented:
"""
Hi Alex, a few comments:

1.) please see PEP8 guide for correct Python formatting https://www.python.org/dev/peps/pep-0008/ namely, do not use tabs but 4 spaces for indentation.

2.) I do not see much value in sorting TXT records. We are searching for _kerberos TXT record which should occur only once in DNS domain.

3.) please use a more concise sorting mechanism mentioned by @tiran, your way is very unpythonic and inefficient (list insertions).
"""

See the full comment at https://github.com/freeipa/freeipa/pull/681#issuecomment-291064621
alex-zel
2017-04-04 05:57:08 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Author: alex-zel
Title: #681: Fix ipadiscovery
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/681/head:pr681
git checkout pr681
alex-zel
2017-04-04 06:06:46 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Author: alex-zel
Title: #681: Fix ipadiscovery
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/681/head:pr681
git checkout pr681
alex-zel
2017-04-04 06:08:13 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/681
Author: alex-zel
Title: #681: Fix ipadiscovery
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/681/head:pr681
git checkout pr681

Loading...