[Freeipa-devel] [freeipa PR#91][+ack] Add help info about certificate revocation reasons

2016-09-19 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/91
Title: #91: Add help info about certificate revocation reasons

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#91][comment] Add help info about certificate revocation reasons

2016-09-19 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/91
Title: #91: Add help info about certificate revocation reasons

stlaz commented:
"""
Please, don't forget to add the ACK labels if you think the code should be 
pushed into FreeIPA.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/91#issuecomment-248208732
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#90][+ack] Update ipa-server-install man page for hostname

2016-09-19 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/90
Title: #90: Update ipa-server-install man page for hostname

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#88][synchronized] test_plugable: update the rest of test_init

2016-09-19 Thread jcholast
   URL: https://github.com/freeipa/freeipa/pull/88
Author: jcholast
 Title: #88: test_plugable: update the rest of test_init
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/88/head:pr88
git checkout pr88
From 5e97b695aaf1f65472cdee6c89ba19b346f07917 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 15 Sep 2016 14:38:49 +0200
Subject: [PATCH] test_plugable: update the rest of test_init

In commit ed4c2d9252a995d01dc098e5b761ded8cd9373d8, changes to the Plugin
class were made, but the test was updated only partially.

Update the rest to fix the failing test.

https://fedorahosted.org/freeipa/ticket/6313
---
 ipatests/test_ipalib/test_plugable.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ipatests/test_ipalib/test_plugable.py b/ipatests/test_ipalib/test_plugable.py
index 0ea02a7..8617c73 100644
--- a/ipatests/test_ipalib/test_plugable.py
+++ b/ipatests/test_ipalib/test_plugable.py
@@ -26,7 +26,7 @@
 
 from ipatests.util import raises, read_only
 from ipatests.util import ClassChecker, create_test_api
-from ipalib import plugable, errors, text
+from ipalib import plugable, errors
 
 import pytest
 
@@ -52,7 +52,7 @@ def test_init(self):
 api = 'the api instance'
 o = self.cls(api)
 assert o.name == 'Plugin'
-assert isinstance(o.doc, text.Gettext)
+assert isinstance(o.doc, str)
 class some_subclass(self.cls):
 """
 Do sub-classy things.
@@ -66,11 +66,12 @@ class some_subclass(self.cls):
 o = some_subclass(api)
 assert o.name == 'some_subclass'
 assert o.summary == 'Do sub-classy things.'
-assert isinstance(o.doc, text.Gettext)
+assert isinstance(o.doc, str)
 class another_subclass(self.cls):
 pass
 o = another_subclass(api)
-assert o.summary == '<%s>' % o.fullname
+assert o.summary == u'<%s.%s>' % (another_subclass.__module__,
+  another_subclass.__name__)
 
 # Test that Plugin makes sure the subclass hasn't defined attributes
 # whose names conflict with the logger methods set in Plugin.__init__():
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#86][comment] Made sssd restart a non-raising opration

2016-09-19 Thread mirielka
  URL: https://github.com/freeipa/freeipa/pull/86
Title: #86: Made sssd restart a non-raising opration

mirielka commented:
"""
Hi,
so sorry about this, but the necessity of sssd restart was caused by some 
leftover mess on the machines where I ran the trust related tests. If the 
configuration is correct and system clean, the restart is not necessary. I 
recommend rejecting this patch and will provide fix for my error ASAP.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/86#issuecomment-248088611
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][+pushed] Keep NSS trust flags of existing certificates

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/76
Title: #76: Keep NSS trust flags of existing certificates

Label: +pushed
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][comment] Keep NSS trust flags of existing certificates

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/76
Title: #76: Keep NSS trust flags of existing certificates

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/2bc70a5d5f5eb953969e7341179c5083c147221a
ipa-4-3:
https://fedorahosted.org/freeipa/changeset/b3e57f789ef7f697f8cc68f180dc8ce292954ed4
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/76#issuecomment-248035107
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][closed] Keep NSS trust flags of existing certificates

2016-09-19 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/76
Author: tomaskrizek
 Title: #76: Keep NSS trust flags of existing certificates
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/76/head:pr76
git checkout pr76
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][comment] Keep NSS trust flags of existing certificates

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/76
Title: #76: Keep NSS trust flags of existing certificates

mbasti-rh commented:
"""
This does not apply to ipa-4-2, please rebase and create a new PR for 4.2
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/76#issuecomment-248034608
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#88][comment] test_plugable: update the rest of test_init

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/88
Title: #88: test_plugable: update the rest of test_init

mbasti-rh commented:
"""
Can you please remove unused import?
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/88#issuecomment-248033023
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#87][closed] dns: re-introduce --raw in dnsrecord-del

2016-09-19 Thread mbasti-rh
   URL: https://github.com/freeipa/freeipa/pull/87
Author: jcholast
 Title: #87: dns: re-introduce --raw in dnsrecord-del
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/87/head:pr87
git checkout pr87
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#87][+pushed] dns: re-introduce --raw in dnsrecord-del

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/87
Title: #87: dns: re-introduce --raw in dnsrecord-del

Label: +pushed
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#87][comment] dns: re-introduce --raw in dnsrecord-del

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/87
Title: #87: dns: re-introduce --raw in dnsrecord-del

mbasti-rh commented:
"""
Fixed upstream
master:
https://fedorahosted.org/freeipa/changeset/e5f7a612fbfdaa9ee12ef16cef550931011abe4c
ipa-4-4:
https://fedorahosted.org/freeipa/changeset/2609a3ef4b5d4f8f043128365baaa4a046967483
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/87#issuecomment-248029245
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#92][opened] Add log messages for IP checks during client install

2016-09-19 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/92
Author: tomaskrizek
 Title: #92: Add log messages for IP checks during client install
Action: opened

PR body:
"""
The added log messages allow easier debugging of
IP related issues during ipa-client-install.

https://fedorahosted.org/freeipa/ticket/6331
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/92/head:pr92
git checkout pr92
From 53ac34988eefe6e43ee4281f23faf55b9ebb9f8a Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Mon, 19 Sep 2016 17:04:18 +0200
Subject: [PATCH] Add log messages for IP checks during client install

The added log messages allow easier debugging of
IP related issues during ipa-client-install.

https://fedorahosted.org/freeipa/ticket/6331
---
 client/ipa-client-install | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index f22e653..24f94d0 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -1569,8 +1569,9 @@ def get_local_ipaddresses(iface=None):
 for ip in if_addrs.get(family, []):
 try:
 ips.append(ipautil.CheckedIPAddress(ip['addr']))
-except ValueError:
-continue
+root_logger.debug('IP check sucessfull: %s' % ip['addr'])
+except ValueError as e:
+root_logger.debug('IP check failed: %s' % str(e))
 return ips
 
 
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#87][+ack] dns: re-introduce --raw in dnsrecord-del

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/87
Title: #87: dns: re-introduce --raw in dnsrecord-del

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#10][comment] Client-side CSR autogeneration

2016-09-19 Thread LiptonB
  URL: https://github.com/freeipa/freeipa/pull/10
Title: #10: Client-side CSR autogeneration

LiptonB commented:
"""
`csrgen` sounds good to me. The new modules have now been moved to 
`ipaclient.plugins.csrgen`, `ipaclient.csrgen`, and 
`ipatests.test_ipaclient.test_csrgen`.

FYI: I force pushed a few minutes after adding these commits to fix a pep8 
error.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/10#issuecomment-247984688
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#87][comment] dns: re-introduce --raw in dnsrecord-del

2016-09-19 Thread mbasti-rh
  URL: https://github.com/freeipa/freeipa/pull/87
Title: #87: dns: re-introduce --raw in dnsrecord-del

mbasti-rh commented:
"""
Works for me with server API.

"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/87#issuecomment-247984671
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][comment] Keep NSS trust flags of existing certificates

2016-09-19 Thread flo-renaud
  URL: https://github.com/freeipa/freeipa/pull/76
Title: #76: Keep NSS trust flags of existing certificates

flo-renaud commented:
"""
(re-sending as setting the review state did not send any email)
Hi Tomas,
thanks for your patch. Works as expected.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/76#issuecomment-247979575
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#76][+ack] Keep NSS trust flags of existing certificates

2016-09-19 Thread flo-renaud
  URL: https://github.com/freeipa/freeipa/pull/76
Title: #76: Keep NSS trust flags of existing certificates

Label: +ack
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#91][opened] Add help info about certificate revocation reasons

2016-09-19 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/91
Author: tomaskrizek
 Title: #91: Add help info about certificate revocation reasons
Action: opened

PR body:
"""
Inform the user where to find additional information
about certificate revocation reasons.

https://fedorahosted.org/freeipa/ticket/6327
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/91/head:pr91
git checkout pr91
From 3d31c7f9956c9329dce4c2cbc5316b9a55a2fbe9 Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Mon, 19 Sep 2016 13:10:20 +0200
Subject: [PATCH] Add help info about certificate revocation reasons

Inform the user where to find additional information
about certificate revocation reasons.

https://fedorahosted.org/freeipa/ticket/6327
---
 ipaserver/plugins/cert.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 68fc2bf..dcc8290 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -826,7 +826,8 @@ class cert(BaseCertObject):
 Int(
 'revocation_reason',
 label=_('Revocation reason'),
-doc=_('Reason for revoking the certificate (0-10)'),
+doc=_('Reason for revoking the certificate (0-10). Type '
+  '"ipa help cert" for revocation reason details. '),
 minvalue=0,
 maxvalue=10,
 flags={'no_create', 'no_update'},
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [freeipa PR#90][opened] Update ipa-server-install man page for hostname

2016-09-19 Thread tomaskrizek
   URL: https://github.com/freeipa/freeipa/pull/90
Author: tomaskrizek
 Title: #90: Update ipa-server-install man page for hostname
Action: opened

PR body:
"""
Hostname is always set, remove the text that says
hostname is set only if it does not match the current
hostname.

https://fedorahosted.org/freeipa/ticket/6330
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/90/head:pr90
git checkout pr90
From 76fba5170eb06f560807304e097af6a0fad6f44f Mon Sep 17 00:00:00 2001
From: Tomas Krizek 
Date: Mon, 19 Sep 2016 12:52:15 +0200
Subject: [PATCH] Update ipa-server-install man page for hostname

Hostname is always set, remove the text that says
hostname is set only if it does not match the current
hostname.

https://fedorahosted.org/freeipa/ticket/6330
---
 install/tools/man/ipa-server-install.1 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 55b4944..f7c1921 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -43,7 +43,7 @@ The password for the IPA admin user
 Create home directories for users on their first login
 .TP
 \fB\-\-hostname\fR=\fIHOST_NAME\fR
-The fully\-qualified DNS name of this server. If the hostname does not match system hostname, the system hostname will be updated accordingly to prevent service failures.
+The fully\-qualified DNS name of this server.
 .TP
 \fB\-\-ip\-address\fR=\fIIP_ADDRESS\fR
 The IP address of this server. If this address does not match the address the host resolves to and \-\-setup\-dns is not selected the installation will fail. If the server hostname is not resolvable, a record for the hostname and IP_ADDRESS is added to /etc/hosts.
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] Github review feature

2016-09-19 Thread Ben Lipton

On 09/16/2016 03:17 AM, Martin Basti wrote:


Sorry for stealing your thread, but you started asking about github 
review emails :)



Standard review inline comments are disabled on purpose, each comment 
generates one email, so we decided that is better after review to 
write a regular comment "NACK, please see inline comments" or so.


I would expecting that the new review feature is sending all comments 
in one batch, but I was wrong. I used the new review feature (with the 
pending comments) but when I sent it I received all comments as single 
notifications again, so again one inline comment = one email


Even worse it is with states of review (approved, required change). I 
didn't received any notification from github related to this (not sure 
if is part of any inline comment message or just not implemented yet). 
This is not documented in their API docs (according David) so we 
cannot use it our tools yet.


Generally adding Labels ACK/Rejected are more visible and filters can 
be made easily.



So for now I would stay with our old workflow and not extend email 
notifications. We can play with new review feature for longer time and 
decide if it is worth to use it (and change email notification 
accordingly)




That all seems reasonable. I hope they will improve the API in the 
future to make this work better, but in the meantime the current process 
is fine. Thanks for looking into it!


Ben

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


[Freeipa-devel] [freeipa PR#10][comment] Client-side CSR autogeneration

2016-09-19 Thread jcholast
  URL: https://github.com/freeipa/freeipa/pull/10
Title: #10: Client-side CSR autogeneration

jcholast commented:
"""
1. I'm afraid `certrequest` (actually `certreq`) is already taken. What about 
`csrgen`?
2. I would be perfectly happy with `ipaclient.`.
3. OK.

Logistical stuff:

- I'm fine with the testing commits being part of this review. I would also be 
fine with new content if it was added after this part of review is done.
- I'm fine with later rebase.
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/10#issuecomment-247930858
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code