Re: [Freeipa-devel] [PATCH 0024] do not log BINDs to non-existent users as errors

2015-04-02 Thread Jan Cholasta

Dne 30.3.2015 v 14:10 Petr Spacek napsal(a):

On 25.3.2015 17:07, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/4889


ACK



Pushed to:
master: 4192cce80eb22172696b11bf24457f7467b711fc
ipa-4-1: ede3298fdf8092567b7cfec4053c0db45725f882

--
Jan Cholasta

--
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] [PATCH 0222] DNSSEC: do not log into files

2015-04-02 Thread Jan Cholasta

Dne 30.3.2015 v 13:55 Petr Spacek napsal(a):

On 26.3.2015 16:33, Martin Basti wrote:

We want to log DNSSEC daemons only into console (journald).

This patch also fixes unexpected log file in
/var/lib/softhsm/.ipa/log/default.log

Patch attached.


ACK



Pushed to:
master: 1216da8b9f2100cacebbeb8fe2dd91e22b954ba7
ipa-4-1: e27b9d18cee86b7634a0ec23042985c23096098e

--
Jan Cholasta

--
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] [PATCH] 809 speed up convert_attribute_members

2015-04-02 Thread Jan Cholasta

Hi,

Dne 31.3.2015 v 12:11 Petr Vobornik napsal(a):

A workaround to avoid usage of slow LDAPEntry._sync_attr #4946.

I originally wanted to avoid DN processing as well but we can't do that
because of DNs which are encoded - e.g. contains '+' or ','. Therefore
patch 811 - faster DN implementation is very useful. Also patch 809 is
useful to avoid high load of 389.

https://fedorahosted.org/freeipa/ticket/4965



1)

+dn = container_dns.get(ldap_obj_name, None)
+if not dn:
+ldap_obj = self.api.Object[ldap_obj_name]
+dn = DN(ldap_obj.container_dn, api.env.basedn)
+container_dns[ldap_obj_name] = dn
+return dn

a) The second argument of .get() is None by default

b) not dn matches None as well as empty DNs, use dn is not None 
(it's not that there could be empty DNs here, but let's not give a 
potential reader the wrong idea)


c) It would be better to catch KeyError rather than call .get() and 
check the result:


try:
dn = container_dns[ldap_obj_name]
except KeyError:
dn = ...
container_dns[ldap_obj_name] = dn


2) Does get_new_attr() actually provide any speed up? Unless I'm missing 
something, it just mirrors the virtual member attributes already readily 
available from entry_attrs in new_attrs.



3) get_container_dn() and get_new_attr() do not need to be functions, 
since each is called just from a single spot.



4) memberdn = DN(member) could be one for loop up.


Here's what I ended up with trying to fix the above (untested):

for attr in self.attribute_members:
try:
value = entry_attrs.raw[attr]
except KeyError:
continue
del entry_attrs[attr]

ldap_objs = {}
for ldap_obj_name in self.attribute_members[attr]:
ldap_obj = self.api.Object[ldap_obj_name]
container_dn = DN(ldap_obj.container_dn, api.env.basedn)
ldap_objs[container_dn] = ldap_obj

for member in value:
memberdn = DN(member)
try:
ldap_obj = ldap_objs[DN(*memberdn[1:])]
except KeyError:
continue

new_attr = '%s_%s' % (attr, ldap_obj.name)
new_value = ldap_obj.get_primary_key_from_dn(memberdn)
entry_attrs.setdefault(new_attr, []).append(new_value)

Honza

--
Jan Cholasta

--
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] [PATCH] 811 performance: faster DN implementation

2015-04-02 Thread Petr Viktorin

On 03/31/2015 12:11 PM, Petr Vobornik wrote:

The only different thing is a lack of utf-8 encoded str support(as
input). I don't know how much important the support is.


I don't think that support is too important (assuming IPA doesn't use 
it!). However, the behavior with this patch is dangerous.
It allows unicode and ASCII strings, but fails on non-ASCII strings. 
That means things will usually work, but as soon as a non-ASCII 
component is introduced at the wrong place, you get an error.


Restoring support for utf-8 encoded str looks easy to do; here's a patch 
you can squash in. Or did I miss something?



maybe it could be attached to ticket
https://fedorahosted.org/freeipa/ticket/4947
-
DN code was optimized to be faster if DNs are created from string. This
is the major use case, since most DNs come from LDAP.

With this patch, DN creation is almost 8-10x faster (with 30K-100K DNs).

Second mojor use case - deepcopy in LDAPEntry is about 20x faster - done
by custom __deepcopy__ function.

The major change is that DN is no longer internally composed  of RDNs
and AVAs but it rather keeps the data in open ldap format - the same as
output of str2dn function. Therefore, for immutable DNs, no other
transformations are required on instantiation.

The format is:

DN: [RDN, RDN,...]
RDN: [AVA, AVA,...]
AVA: ['utf-8 encoded str - attr', 'utf-8 encode str -value', FLAG]
FLAG: int

Further indexing of DN object constructs an RDN which is just an
encapsulation of the RDN part of open ldap representation. Indexing of
RDN constructs AVA in the same fashion.

Obtained EditableAVA, EditableRDN from EditableDN shares the respected
lists of the open ldap repr. so that the change of value or attr is
reflected in parent object.



Looks good. A couple of comments:

RDN.to_openldap: _avas always has 3 components, right? I'd prefer 
`list(a)` over `[a[0], a[1], a[2]]`.  Similarly for tuple in in __add__ 
and RDN._avas_from_sequence.


DN._rdns_from_value: the error message at the end is wrong, RDN is also 
accepted. (And, `type(value)` would be more informative than 
`value.__class__.__name__`.)


You can optimize __deepcopy__ for immutable DNs even further: just 
return self!


In DN.find  rfind, RDNs are not accepted but the error message says 
they are.


You removed the newline at end of file.

--
Petr Viktorin

From 1461bc75748d5bdcf301242c3e31c044542f5bdd Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 2 Apr 2015 10:40:15 +0200
Subject: [PATCH] Restore support for creating DNs from utf-8 encoded strings

---
 ipapython/dn.py|  4 ++-
 ipatests/test_ipapython/test_dn.py | 60 +++---
 2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/ipapython/dn.py b/ipapython/dn.py
index 5d1f15c..dc536ce 100644
--- a/ipapython/dn.py
+++ b/ipapython/dn.py
@@ -1225,7 +1225,9 @@ def _copy_rdns(self, rdns=None):
 def _rdns_from_value(self, value):
 if isinstance(value, basestring):
 try:
-rdns = str2dn(value.encode('utf-8'))
+if isinstance(value, unicode):
+value = value.encode('utf-8')
+rdns = str2dn(value)
 if self.is_mutable:
 self._copy_rdns(rdns)  # AVAs to be list instead of tuple
 except DECODING_ERROR:
diff --git a/ipatests/test_ipapython/test_dn.py b/ipatests/test_ipapython/test_dn.py
index 2a7739b..394be5b 100644
--- a/ipatests/test_ipapython/test_dn.py
+++ b/ipatests/test_ipapython/test_dn.py
@@ -1864,11 +1864,11 @@ def test_i18n(self):
 self.assertEqual(ava1.attr, self.arabic_hello_unicode)
 self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo')
 
-# ava1 = AVA_class(self.arabic_hello_utf8, 'foo')
-# self.assertIsInstance(ava1.attr,  unicode)
-# self.assertIsInstance(ava1.value, unicode)
-# self.assertEqual(ava1.attr, self.arabic_hello_unicode)
-# self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo')
+ava1 = AVA_class(self.arabic_hello_utf8, 'foo')
+self.assertIsInstance(ava1.attr,  unicode)
+self.assertIsInstance(ava1.value, unicode)
+self.assertEqual(ava1.attr, self.arabic_hello_unicode)
+self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo')
 
 # test value i18n
 ava1 = AVA_class('cn', self.arabic_hello_unicode)
@@ -1877,11 +1877,11 @@ def test_i18n(self):
 self.assertEqual(ava1.value, self.arabic_hello_unicode)
 self.assertEqual(str(ava1), 'cn='+self.arabic_hello_utf8)
 
-# ava1 = AVA_class('cn', self.arabic_hello_utf8)
-# self.assertIsInstance(ava1.attr,  unicode)
-# self.assertIsInstance(ava1.value, unicode)
-# self.assertEqual(ava1.value, self.arabic_hello_unicode)
-# self.assertEqual(str(ava1), 

Re: [Freeipa-devel] [PATCH 0212] Server Upgrade: Fix comments

2015-04-02 Thread Jan Cholasta

Dne 24.3.2015 v 14:23 David Kupka napsal(a):

On 03/24/2015 12:04 PM, Martin Basti wrote:

On 24/03/15 09:54, Martin Basti wrote:

On 23/03/15 15:36, Martin Basti wrote:

Attached patch fixes comments which I forgot to edit in 'make upgrade
deterministic' patchset




I missed some dictionaries which should be lists.

Updated patch attached.

--
Martin Basti



Updated patch attached



Thanks for the patch, LGTM, ACK.



Pushed to master: b5e941d49b3571a3f257be645dabb429754c94b0

--
Jan Cholasta

--
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] [PATCH] 0001-2 ipatests: SOA record Maintenance tests

2015-04-02 Thread Jan Cholasta

Dne 27.3.2015 v 16:54 Martin Basti napsal(a):

On 27/03/15 16:34, Aleš Mareček wrote:

Greetings!
Martin, thanks for your review and comments!
I changed the name of the patch and setup my git variables properly. I
also re-tested it and got all passed. I'm sending a new patch that is
attached.

- Original Message -

From: Martin Basti mba...@redhat.com
To: Aleš Mareček amare...@redhat.com, freeipa-devel@redhat.com
Sent: Tuesday, March 24, 2015 4:39:21 PM
Subject: Re: [Freeipa-devel] [PATCH] 0001 ipatests: SOA record
Maintenance tests

On 24/03/15 15:06, Aleš Mareček wrote:

Greetings!
This is my very first patch, ticket#4746.

Have a nice day!
   - alich -



Thank you for the patch. Just nitpicks:

1)
+cleanup_commands = [
+('dnszone_del', [zone6], {'continue': True}),
+('dnszone_del', [zone6b], {'continue': True}),
+]

would be better do it in this way, continue option will to try remove
all zones:
+cleanup_commands = [
+('dnszone_del', [zone6, zone6b], {'continue': True}),
+]


Done.


2)
I'm fine with zone6b, but was there any reason to create zone6b, instead
of reusing zone 1 or 2 or 3?

Because of some updates needs, I didn't want to break anything
existing thus I created new.


3)
Please fix whitespace errors.
$ git am
freeipa-alich-0001-ipatests-added-tests-for-SOA-record-Maintenance.patch
Applying: ipatests - added tests for SOA record Maintenance
/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:482: trailing
whitespace.

/home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:758: new blank
line at EOF.
+
warning: 2 lines add whitespace errors.


Done.
$ git am freeipa-alich-0001-2-Ipatests-DNS-SOA-Record-Maintenance.patch
Applying: Ipatests DNS SOA Record Maintenance
$


4)
I know the dns plugin tests are so far from PEP8, but try to keep PEP8
in new code

Done, only 1 line persisted that I didn't want to break:
zone6_unresolvable_ns_relative_dnsname =
DNSName(zone6_unresolvable_ns_relative)


Otherwise test works as expected.

Martin^2

--
Martin Basti



Thanks!
  - alich -

Thank you, ACK.



Pushed to:
master: ca96ecbf40038d09814f99f19bf47246352dfa0c
ipa-4-1: 8f94ac1e7c24b3bf33c5211d3e327c9a51390fb1

--
Jan Cholasta

--
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] [PATCH 0021] fix improper handling of boolean option during KRA install

2015-04-02 Thread Jan Cholasta

Dne 25.3.2015 v 16:48 Martin Basti napsal(a):

On 17/03/15 17:51, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/4530




ACK

--
Martin Basti





Pushed to master: c311af06f60cfdb73be9c0aecb9ddc559db1a055

--
Jan Cholasta

--
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] [PATCH 0223] Fix ldap2 do not create shared instance by default

2015-04-02 Thread Martin Basti

On 02/04/15 14:11, Jan Cholasta wrote:

Hi,

Dne 1.4.2015 v 17:16 Martin Basti napsal(a):

Since API is not singleton anymore, ldap2 instance should not be shared
between all APIs.

Patch attached.



Works for me. However, it's not the ldap2 instance that was shared, 
but rather the underlying LDAP connection.


Honza


Reworded patch attached.

--
Martin Basti

From fde4f6434fa746c69ca97b68bbf7e8d08448c21e Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 25 Mar 2015 15:34:16 +0100
Subject: [PATCH] Fix ldap2 shared connection

Since API is not singleton anymore, ldap2 connections should not be
shared by default.
---
 ipalib/backend.py|  2 +-
 ipaserver/plugins/ldap2.py   |  2 +-
 ipatests/test_ipalib/test_backend.py | 12 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ipalib/backend.py b/ipalib/backend.py
index 4c1001d4d47613537b64c314a2d22769a27f4c69..fcbbd254afc797019e9ea63214b1ee034b8c13f8 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -46,7 +46,7 @@ class Connectible(Backend):
 `request.destroy_context()` can properly close all open connections.
 
 
-def __init__(self, shared_instance=True):
+def __init__(self, shared_instance=False):
 Backend.__init__(self)
 if shared_instance:
 self.id = self.name
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 3211b3390fb979f090467445905513d33e537e17..fd4ed29903fb2f3afe0f4b74467bf53df49654fa 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -61,7 +61,7 @@ class ldap2(LDAPClient, CrudBackend):
 LDAP Backend Take 2.
 
 
-def __init__(self, shared_instance=True, ldap_uri=None, base_dn=None,
+def __init__(self, shared_instance=False, ldap_uri=None, base_dn=None,
  schema=None):
 self.__ldap_uri = None
 
diff --git a/ipatests/test_ipalib/test_backend.py b/ipatests/test_ipalib/test_backend.py
index c69757cb3d68ebc12f9c91572d37603738357c4e..121c4745bd1dfebfbeed75ba1b46b4420064fe63 100644
--- a/ipatests/test_ipalib/test_backend.py
+++ b/ipatests/test_ipalib/test_backend.py
@@ -76,7 +76,7 @@ class test_Connectible(ClassChecker):
 object.__setattr__(self, 'args', args)
 object.__setattr__(self, 'kw', kw)
 return 'The connection.'
-o = example()
+o = example(shared_instance=True)
 args = ('Arg1', 'Arg2', 'Arg3')
 kw = dict(key1='Val1', key2='Val2', key3='Val3')
 assert not hasattr(context, 'example')
@@ -104,7 +104,7 @@ class test_Connectible(ClassChecker):
 class example(self.cls):
 pass
 for klass in (self.cls, example):
-o = klass()
+o = klass(shared_instance=True)
 e = raises(NotImplementedError, o.create_connection)
 assert str(e) == '%s.create_connection()' % klass.__name__
 
@@ -114,7 +114,7 @@ class test_Connectible(ClassChecker):
 
 class example(self.cls):
 destroy_connection = Disconnect()
-o = example()
+o = example(shared_instance=True)
 
 m = disconnect: 'context.%s' does not exist in thread %r
 e = raises(StandardError, o.disconnect)
@@ -131,7 +131,7 @@ class test_Connectible(ClassChecker):
 class example(self.cls):
 pass
 for klass in (self.cls, example):
-o = klass()
+o = klass(shared_instance=True)
 e = raises(NotImplementedError, o.destroy_connection)
 assert str(e) == '%s.destroy_connection()' % klass.__name__
 
@@ -142,7 +142,7 @@ class test_Connectible(ClassChecker):
 class example(self.cls):
 pass
 for klass in (self.cls, example):
-o = klass()
+o = klass(shared_instance=True)
 assert o.isconnected() is False
 conn = 'whatever'
 setattr(context, klass.__name__, conn)
@@ -157,7 +157,7 @@ class test_Connectible(ClassChecker):
 class example(self.cls):
 pass
 for klass in (self.cls, example):
-o = klass()
+o = klass(shared_instance=True)
 e = raises(AttributeError, getattr, o, 'conn')
 assert str(e) == msg % (
 klass.__name__, threading.currentThread().getName()
-- 
2.1.0

-- 
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] [PATCH 0223] Fix ldap2 do not create shared instance by default

2015-04-02 Thread Jan Cholasta

Dne 2.4.2015 v 14:18 Martin Basti napsal(a):

On 02/04/15 14:11, Jan Cholasta wrote:

Hi,

Dne 1.4.2015 v 17:16 Martin Basti napsal(a):

Since API is not singleton anymore, ldap2 instance should not be shared
between all APIs.

Patch attached.



Works for me. However, it's not the ldap2 instance that was shared,
but rather the underlying LDAP connection.

Honza


Reworded patch attached.



Thanks, ACK.

Pushed to master: b92136cba287e38d9c2f41c3163f5a6b0b62ca17

--
Jan Cholasta

--
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] [PATCH] 0688 rename_managed: Remove use of EditableDN

2015-04-02 Thread Petr Viktorin

This removes the last use of dn.Editable* from IPA code.
For cases where an EditableDN was used, lists/generators of RDNs tend to 
work better IMO.


When this is merged, Editable* can be removed (which I don't want to do 
right now since there's a patch on review that would conflict).


--
Petr Viktorin
From 16fee8dd83d71789e97e85a88993d499cca34d9c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 1 Apr 2015 16:37:51 +0200
Subject: [PATCH] rename_managed: Remove use of EditableDN

This was the last use of EditableDN in IPA; the class can now be removed.
---
 ipaserver/install/plugins/rename_managed.py | 51 ++---
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/ipaserver/install/plugins/rename_managed.py b/ipaserver/install/plugins/rename_managed.py
index adb814c1799ebbdb57118acf1ba6a52550f2f818..c5e701a588bf34b0f86de197859e9c55e8db5dd0 100644
--- a/ipaserver/install/plugins/rename_managed.py
+++ b/ipaserver/install/plugins/rename_managed.py
@@ -21,7 +21,7 @@
 from ipaserver.install.plugins.baseupdate import PreUpdate, PostUpdate
 from ipalib import api, errors
 from ipapython import ipautil
-from ipapython.dn import DN, EditableDN
+from ipapython.dn import DN
 
 def entry_to_update(entry):
 
@@ -41,7 +41,21 @@ def entry_to_update(entry):
 
 return update
 
+
 class GenerateUpdateMixin(object):
+def _dn_replace(self, dn, old, new):
+Replace all occurences of old AVAs in a DN by new
+
+If a replacement is made, return the resulting DN.
+Otherwise, log, an raise ValueError.
+
+new_dn = DN(new if ava == old else ava for ava in dn)
+if new_dn == dn:
+self.error(unable to replace '%s' with '%s' in '%s',
+   old, new, dn)
+raise ValueError('no replacement made')
+return new_dn
+
 def generate_update(self, deletes=False):
 
 We need to separate the deletes that need to happen from the
@@ -82,14 +96,13 @@ def generate_update(self, deletes=False):
 pass
 else:
 # Compute the new dn by replacing the old container with the new container
-new_dn = EditableDN(entry.dn)
-if new_dn.replace(old_template_container, new_template_container) != 1:
-self.error(unable to replace '%s' with '%s' in '%s',
-   old_template_container, new_template_container, entry.dn)
+try:
+new_dn = self._dn_replace(entry.dn,
+  old=old_template_container,
+  new=new_template_container)
+except ValueError:
 continue
 
-new_dn = DN(new_dn)
-
 # The old attributes become defaults for the new entry
 new_update = {'dn': new_dn,
   'default': entry_to_update(entry)}
@@ -103,23 +116,21 @@ def generate_update(self, deletes=False):
 
 else:
 # Update the template dn by replacing the old containter with the new container
-old_dn = entry['managedtemplate'][0]
-new_dn = EditableDN(old_dn)
-if new_dn.replace(old_template_container, new_template_container) != 1:
-self.error(unable to replace '%s' with '%s' in '%s',
-   old_template_container, new_template_container, old_dn)
+try:
+new_dn = self._dn_replace(entry['managedtemplate'][0],
+  old=old_template_container,
+  new=new_template_container)
+except ValueError:
 continue
-new_dn = DN(new_dn)
 entry['managedtemplate'] = new_dn
 
-# Edit the dn, then convert it back to an immutable DN
-old_dn = entry.dn
-new_dn = EditableDN(old_dn)
-if new_dn.replace(old_definition_container, new_definition_container) != 1:
-self.error(unable to replace '%s' with '%s' in '%s',
-   old_definition_container, new_definition_container, old_dn)
+# Update the entry dn similarly
+try:
+new_dn = self._dn_replace(entry.dn,
+  old=old_definition_container,
+  new=new_definition_container)
+except ValueError:
 continue
-new_dn = DN(new_dn)
 
 # The old attributes become defaults for the new entry
 new_update = {'dn': new_dn,
-- 
2.1.0

-- 
Manage your subscription for the 

Re: [Freeipa-devel] [PATCH 0045] Add message for skipping NTP configuration during client install

2015-04-02 Thread Gabe Alford
On Thu, Apr 2, 2015 at 8:59 AM, Martin Basti mba...@redhat.com wrote:

  On 30/03/15 15:25, Gabe Alford wrote:

   Hello,

  With the merging of ticket 4842
 https://fedorahosted.org/freeipa/ticket/4842, I believe that half of
 ticket 3092 https://fedorahosted.org/freeipa/ticket/3092 has been done.
 This patch just adds a message that says that NTP configuration was skipped
 which I believe should finish 3092
 https://fedorahosted.org/freeipa/ticket/3092.

  Thanks,

  Gabe


  Hello, thank you for the patch.

 1)
 IMO there should be:
 if *not* options.conf_ntp


So, if --no-ntp is not specified, print message that the client is skipping
NTP sync?


 2)
 wouldnt be better to use just else?


I actually ran ipa-client-install with no options on a system where I used
'else', and it printed the skipping NTP sync when it should not have.
That is why the patch does not use 'else'.



 Martin

 --
 Martin Basti


-- 
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] [PATCH 0045] Add message for skipping NTP configuration during client install

2015-04-02 Thread Martin Basti

On 30/03/15 15:25, Gabe Alford wrote:

Hello,

With the merging of ticket 4842 
https://fedorahosted.org/freeipa/ticket/4842, I believe that half of 
ticket 3092 https://fedorahosted.org/freeipa/ticket/3092 has been 
done. This patch just adds a message that says that NTP configuration 
was skipped which I believe should finish 3092 
https://fedorahosted.org/freeipa/ticket/3092.


Thanks,

Gabe



Hello, thank you for the patch.

1)
IMO there should be:
if *not* options.conf_ntp

2)
wouldnt be better to use just else?

Martin

--
Martin Basti

-- 
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