Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


Hello!

Patches:
client: add support for pre-schema servers
client: do not crash when overriding remote command as method

brings compatibility with old servers into thin client (making it not so 
thin after all :-). I've tested it against 4.2 and 4.3 servers and the 
important parts work. There are some minor issues (e.g. dynamic defaults 
not working) but they can be addressed later, ACK.



--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 30.6.2016 16:25, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

server: exclude Local commands from RPC
client: add placeholders for required remote plugins
client: ignore override errors in command overrides
plugable: add option to ignore override errors
cert: fix CLI output of cert_remove_hold
frontend: do not ignore client-side output params
user: add object plugin for user_status
server: define missing virtual attributes

contains mostly fixes for some bugs discovered in thin client. Works for
me, ACK.


Thanks, pushed to master: 2beb72ffa4bea5e22c2ba4685a524df36d1f800c

Attaching the patches for reference.

--
Jan Cholasta
From d31aa24657f5fee5cc5498fe8e43fe3926d36f6f Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 06:37:16 +0200
Subject: [PATCH 1/8] server: define missing virtual attributes

Move virtual attributes defined in output params of methods into params of
the related object.

This fixes the virtual attributes being ommited in CLI output.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/plugins/aci.py | 20 +++-
 ipaserver/plugins/baseuser.py|  7 ++--
 ipaserver/plugins/certprofile.py | 10 +++---
 ipaserver/plugins/delegation.py  | 14 +++-
 ipaserver/plugins/dns.py | 21 +++-
 ipaserver/plugins/host.py| 70 +++-
 ipaserver/plugins/idviews.py | 24 +++---
 ipaserver/plugins/otptoken.py|  8 ++---
 ipaserver/plugins/permission.py  | 15 +++--
 ipaserver/plugins/selfservice.py | 15 +++--
 ipaserver/plugins/service.py | 63 
 ipaserver/plugins/trust.py   | 46 ++
 12 files changed, 147 insertions(+), 166 deletions(-)

diff --git a/ipaserver/plugins/aci.py b/ipaserver/plugins/aci.py
index dd14d82..5647827 100644
--- a/ipaserver/plugins/aci.py
+++ b/ipaserver/plugins/aci.py
@@ -507,6 +507,10 @@ class aci(Object):
  flags=('virtual_attribute',),
 ),
 _prefix_option,
+Str('aci',
+label=_('ACI'),
+flags={'no_create', 'no_update', 'no_search'},
+),
 )
 
 
@@ -611,11 +615,6 @@ class aci_mod(crud.Update):
 Modify ACI.
 """
 NO_CLI = True
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
 
 takes_options = (_prefix_option,)
 
@@ -886,12 +885,6 @@ class aci_show(crud.Retrieve):
 """
 NO_CLI = True
 
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
-
 takes_options = (
 _prefix_option,
 DNParam('location?',
@@ -932,11 +925,6 @@ class aci_rename(crud.Update):
 Rename an ACI.
 """
 NO_CLI = True
-has_output_params = (
-Str('aci',
-label=_('ACI'),
-),
-)
 
 takes_options = (
 _prefix_option,
diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py
index 7bb2e8a..8087418 100644
--- a/ipaserver/plugins/baseuser.py
+++ b/ipaserver/plugins/baseuser.py
@@ -59,9 +59,6 @@ baseuser_output_params = (
 Flag('has_keytab',
 label=_('Kerberos keys available'),
 ),
-Str('sshpubkeyfp*',
-label=_('SSH public key fingerprint'),
-),
)
 
 status_baseuser_output_params = (
@@ -353,6 +350,10 @@ class baseuser(LDAPObject):
 normalizer=normalize_sshpubkey,
 flags=['no_search'],
 ),
+Str('sshpubkeyfp*',
+label=_('SSH public key fingerprint'),
+flags={'virtual_attribute', 'no_create', 'no_update', 'no_search'},
+),
 StrEnum('ipauserauthtype*',
 cli_name='user_auth_type',
 label=_('User authentication types'),
diff --git a/ipaserver/plugins/certprofile.py b/ipaserver/plugins/certprofile.py
index 6f314e1..f446607 100644
--- a/ipaserver/plugins/certprofile.py
+++ b/ipaserver/plugins/certprofile.py
@@ -122,6 +122,10 @@ class certprofile(LDAPObject):
 label=_('Profile ID'),
 doc=_('Profile ID for referring to this profile'),
 ),
+Str('config',
+label=_('Profile configuration'),
+flags={'virtual_attribute', 'no_create', 'no_update', 'no_search'},
+),
 Str('description',
 required=True,
 cli_name='desc',
@@ -195,12 +199,6 @@ class certprofile_find(LDAPSearch):
 class certprofile_show(LDAPRetrieve):
 __doc__ = _("Display the properties of a Certificate Profile.")
 
-has_output_params = LDAPRetrieve.has_output_params + (
-Str('config',
-

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

server: exclude Local commands from RPC
client: add placeholders for required remote plugins
client: ignore override errors in command overrides
plugable: add option to ignore override errors
cert: fix CLI output of cert_remove_hold
frontend: do not ignore client-side output params
user: add object plugin for user_status
server: define missing virtual attributes

contains mostly fixes for some bugs discovered in thin client. Works for 
me, ACK.


--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 30.6.2016 10:51, David Kupka wrote:

On 30/06/16 10:45, Jan Cholasta wrote:

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.


Works for me, ACK.


Thanks.

Pushed to master: 8d5272e68775046db450d860c173a4d531a95ff2

--
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] [WIP] Thin client

[David Kupka]

On 30/06/16 10:45, Jan Cholasta wrote:

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.


Works for me, ACK.

--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 30.6.2016 10:32, Jan Cholasta wrote:

On 29.6.2016 10:21, Jan Cholasta wrote:

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.


Martin found a regression caused by this patch. The attached patch
should fix it.


Self-NACK, correct patch attached.

--
Jan Cholasta
From 195dd2f8acf9e7461e25b505a2a6a7d2067765ed Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 10:27:05 +0200
Subject: [PATCH] schema: properly fix Flag arguments on the client

The previous fix in commit a77e21cbca05be422fe5826857cfba7e0ba6e71f made
some Bool arguments appear as Flag on the client. This change fixes that.

https://fedorahosted.org/freeipa/ticket/6009
---
 ipaclient/remote_plugins/schema.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 7d5c8d0..da917a9 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -219,8 +219,8 @@ class _SchemaPlugin(object):
 cls = Password
 sensitive = False
 elif (type_name == 'bool' and
-'default' in schema and
-schema['default'][0] == u'False'):
+'default' in schema and schema['default'][0] == u'False' and
+not schema.get('alwaysask', False)):
 cls = Flag
 del schema['default']
 else:
-- 
2.7.4

-- 
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] [WIP] Thin client

[Jan Cholasta]

On 29.6.2016 10:21, Jan Cholasta wrote:

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.


Martin found a regression caused by this patch. The attached patch 
should fix it.


--
Jan Cholasta
From 61028180135f1f5967778fba6ecabd28f8b2da7d Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 30 Jun 2016 10:27:05 +0200
Subject: [PATCH] schema: properly fix Flag arguments on the client

The previous fix in commit a77e21cbca05be422fe5826857cfba7e0ba6e71f made
some Bool arguments appear as Flag on the client. This change fixes that.

https://fedorahosted.org/freeipa/ticket/6009
---
 ipaclient/remote_plugins/schema.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 7d5c8d0..08ed2b7 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -219,8 +219,8 @@ class _SchemaPlugin(object):
 cls = Password
 sensitive = False
 elif (type_name == 'bool' and
-'default' in schema and
-schema['default'][0] == u'False'):
+'default' in schema and schema['default'][0] == u'False' and
+schema.get('alwaysask', u'False') == u'False'):
 cls = Flag
 del schema['default']
 else:
-- 
2.7.4

-- 
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] [WIP] Thin client

[Jan Cholasta]

On 27.6.2016 16:44, Jan Cholasta wrote:

On 27.6.2016 14:55, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.


Thanks, pushed to master: f7cc15f0990ef2db57717a3c6a8e9db2c3dee951

Attaching the patches for reference.



*) xmlrpc test for automember_plugin test started failing with
automember patch. Fix for test attached.


LGTM


ACK.

Pushed to master: 95191e1612f93823bd0e325ef9b97fa9a4d7869c

--
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] [WIP] Thin client

[Jan Cholasta]

On 29.6.2016 09:22, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


Thanks, pushed to master: a77e21cbca05be422fe5826857cfba7e0ba6e71f

Attaching the patch for reference.

--
Jan Cholasta
From d61e4aff7e3ac4c01e32428768f01928b2fce023 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 28 Jun 2016 08:40:48 +0200
Subject: [PATCH] schema: fix Flag arguments on the client

Fix Flag arguments appearing as Bool on the client.

https://fedorahosted.org/freeipa/ticket/6009
---
 ipaclient/remote_plugins/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a54d4eb..7d5c8d0 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -220,7 +220,7 @@ class _SchemaPlugin(object):
 sensitive = False
 elif (type_name == 'bool' and
 'default' in schema and
-schema['default'] == [u'False']):
+schema['default'][0] == u'False'):
 cls = Flag
 del schema['default']
 else:
-- 
2.7.4

-- 
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] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Commit "schema: fix Flag arguments on the client" fixes regression 
reported in https://fedorahosted.org/freeipa/ticket/6009, ACK.


--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 27.6.2016 14:55, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.


Thanks, pushed to master: f7cc15f0990ef2db57717a3c6a8e9db2c3dee951

Attaching the patches for reference.



*) xmlrpc test for automember_plugin test started failing with
automember patch. Fix for test attached.


LGTM

--
Jan Cholasta
From fd0553ca1216e68edb7ae88997a321339b08b612 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 22 Jun 2016 15:15:32 +0200
Subject: [PATCH 1/4] schema: fix param default value handling

Advertise param's default value even when `autofill` is False. When
`autofill` is False, set `alwaysask` to True in the schema, as it is
semantically equivallent and removes redundancy.

This fixes default value disappearing in CLI for some params.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py |  6 +++---
 ipaserver/plugins/schema.py| 23 +--
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index c21da5f..b944fb0 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -213,7 +213,6 @@ def _create_param(meta):
 
 for key, value in meta.items():
 if key in ('alwaysask',
-   'autofill',
'doc',
'label',
'multivalue',
@@ -229,11 +228,9 @@ def _create_param(meta):
 kwargs[key] = value
 elif key == 'default':
 default = value
-kwargs['autofill'] = True
 elif key == 'default_from_param':
 kwargs['default_from'] = DefaultFrom(_nope,
  *(str(k) for k in value))
-kwargs['autofill'] = True
 elif key in ('exclude',
  'include'):
 kwargs[key] = tuple(str(v) for v in value)
@@ -246,6 +243,9 @@ def _create_param(meta):
 default = tmp._convert_scalar(default[0])
 kwargs['default'] = default
 
+if 'default' in kwargs or 'default_from' in kwargs:
+kwargs['autofill'] = not kwargs.pop('alwaysask', False)
+
 param = cls(str(meta['name']), **kwargs)
 
 if sensitive:
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index a67d7b2..c3a0e60 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -542,23 +542,26 @@ class param(BaseParam):
  'include'):
 obj[key] = list(unicode(v) for v in value)
 if isinstance(metaobj, Command):
-if key in ('alwaysask',
-   'confirm'):
+if key == 'alwaysask':
+obj.setdefault(key, value)
+elif key == 'confirm':
 obj[key] = value
 elif key in ('cli_metavar',
  'cli_name',
  'option_group'):
 obj[key] = unicode(value)
 elif key == 'default':
-if param.autofill:
-if param.multivalue:
-obj[key] = [unicode(v) for v in value]
-else:
-obj[key] = [unicode(value)]
+if param.multivalue:
+obj[key] = [unicode(v) for v in value]
+else:
+obj[key] = [unicode(value)]
+if not param.autofill:
+obj['alwaysask'] = True
 elif key == 'default_from':
-if param.autofill:
-obj['default_from_param'] = list(unicode(k)
- for k in value.keys)
+obj['default_from_param'] = list(unicode(k)
+ for k in value.keys)
+if not param.autofill:
+obj['alwaysask'] = True
 elif key in ('exponential',
  'normalizer',
  'only_absolute',
-- 
2.7.4

From 294bc271dafb021730174cfc4eacaaf187a53e68 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 23 Jun 2016 12:14:38 +0200
Subject: [PATCH 2/4] schema: do not crash in command_defaults if argument is
 None

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:
schema: client-side cleanup
automember: fix automember to work with thin client
schema: do not crash in command_defaults if argument is None
schema: fix param default value handling

works* for me, ACK.

*) xmlrpc test for automember_plugin test started failing with 
automember patch. Fix for test attached.


--
David Kupka
From e736f1d1c9e5247b95fab49a5ea8fa15d6077981 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Mon, 27 Jun 2016 14:52:27 +0200
Subject: [PATCH] test: automember: Fix expected exception message

---
 ipatests/test_xmlrpc/test_automember_plugin.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py
index 9060b69bd57aa8a8a2704dfd233ff3fa75da715a..ffbc91104ab504a98099babb024f9edab114ac5b 100644
--- a/ipatests/test_xmlrpc/test_automember_plugin.py
+++ b/ipatests/test_xmlrpc/test_automember_plugin.py
@@ -196,7 +196,7 @@ class TestNonexistentAutomember(XMLRPC_test):
 group1.ensure_missing()
 command = automember_group.make_delete_command()
 with raises_exact(errors.NotFound(
-reason=u': Automember rule not found')):
+reason=u'%s: Automember rule not found' % group1.cn)):
 command()
 
 def test_create_with_nonexistent_hostgroup(self, automember_hostgroup,
@@ -214,7 +214,7 @@ class TestNonexistentAutomember(XMLRPC_test):
 hostgroup1.ensure_missing()
 command = automember_hostgroup.make_delete_command()
 with raises_exact(errors.NotFound(
-reason=u': Automember rule not found')):
+reason=u'%s: Automember rule not found' % hostgroup1.cn)):
 command()
 
 
-- 
2.7.4

-- 
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] [WIP] Thin client

[Martin Basti]

On 21.06.2016 07:53, Jan Cholasta wrote:

On 20.6.2016 19:56, Martin Basti wrote:



On 20.06.2016 18:48, Martin Basti wrote:



On 20.06.2016 16:42, Jan Cholasta wrote:

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client
regression" yet, I would like Martin to review it first.


ACK

pushed to master:
* 91d6d87ca76e3aa27d5f87fd4f0b70f1d4fe4e72 replica install: fix thin
client regression



Attaching the patches for reference.



Note: There is one one known issue in automember output, will be 
fixed

later.








Guys, I suspect that one of previously pushed patches breaks following
command on the client side

# ipa dns-update-system-records
ipa: ERROR: KeyError: 'ipa_records'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1346, 
in run

sys.exit(api.Backend.cli.run(argv))
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line , 
in run

rv = cmd.output_for_cli(self.api.Backend.textui, result, *args,
**options)
  File "/usr/lib/python2.7/site-packages/ipaclient/plugins/dns.py", line
367, in output_for_cli
textui.print_indented(u'{}:'.format(labels[key]), indent=1)
KeyError: 'ipa_records'
ipa: ERROR: an internal error has occurred


Sorry. Fixed.



ACK

Pushed to master: 894be1bd50905b86d87244d0ede3f266e9737b9a

--
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] [WIP] Thin client

[Jan Cholasta]

On 20.6.2016 19:56, Martin Basti wrote:



On 20.06.2016 18:48, Martin Basti wrote:



On 20.06.2016 16:42, Jan Cholasta wrote:

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client
regression" yet, I would like Martin to review it first.


ACK

pushed to master:
* 91d6d87ca76e3aa27d5f87fd4f0b70f1d4fe4e72 replica install: fix thin
client regression



Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.








Guys, I suspect that one of previously pushed patches breaks following
command on the client side

# ipa dns-update-system-records
ipa: ERROR: KeyError: 'ipa_records'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1346, in run
sys.exit(api.Backend.cli.run(argv))
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line , in run
rv = cmd.output_for_cli(self.api.Backend.textui, result, *args,
**options)
  File "/usr/lib/python2.7/site-packages/ipaclient/plugins/dns.py", line
367, in output_for_cli
textui.print_indented(u'{}:'.format(labels[key]), indent=1)
KeyError: 'ipa_records'
ipa: ERROR: an internal error has occurred


Sorry. Fixed.

--
Jan Cholasta
From a1ae77ff2caf4daa87a0f1fdd4bc5e75bb8c3aae Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 21 Jun 2016 07:47:52 +0200
Subject: [PATCH] dns: fix dns_update_system_records to work with thin client

https://fedorahosted.org/freeipa/ticket/2008
https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt  |  6 --
 VERSION  |  4 ++--
 ipaclient/plugins/dns.py |  4 ++--
 ipaserver/plugins/dns.py | 20 
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/API.txt b/API.txt
index f2a0686..558be34 100644
--- a/API.txt
+++ b/API.txt
@@ -1068,10 +1068,12 @@ output: Output('result', type=[])
 output: Output('summary', type=[, ])
 output: Output('value', type=[])
 command: dns_update_system_records
-args: 0,2,2
+args: 0,4,2
+option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Flag('dry_run', autofill=True, default=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
-output: Output('result', type=[])
+output: Entry('result')
 output: Output('value', type=[])
 command: dnsconfig_mod
 args: 0,11,3
diff --git a/VERSION b/VERSION
index faf10e3..76b83ee 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=193
-# Last change: schema: remove `no_cli` from command schema
+IPA_API_VERSION_MINOR=194
+# Last change: dns: fix dns_update_system_records to work with thin client
diff --git a/ipaclient/plugins/dns.py b/ipaclient/plugins/dns.py
index d04f686..bca5ad7 100644
--- a/ipaclient/plugins/dns.py
+++ b/ipaclient/plugins/dns.py
@@ -23,7 +23,7 @@ from __future__ import print_function
 import six
 import copy
 
-from ipaclient.frontend import MethodOverride, CommandOverride
+from ipaclient.frontend import MethodOverride
 from ipalib import errors
 from ipalib.dns import (get_part_rrtype,
 get_record_rrtype,
@@ -347,7 +347,7 @@ class dnsforwardzone_mod(MethodOverride):
 
 
 @register(override=True)
-class dns_update_system_records(CommandOverride):
+class dns_update_system_records(MethodOverride):
 def output_for_cli(self, textui, output, *args, **options):
 output_super = copy.deepcopy(output)
 super_res = output_super.get('result', {})
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index 14607b3..f350450 100644
--- 

Re: [Freeipa-devel] [WIP] Thin client

[Martin Basti]

On 20.06.2016 18:48, Martin Basti wrote:



On 20.06.2016 16:42, Jan Cholasta wrote:

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client 
regression" yet, I would like Martin to review it first.


ACK

pushed to master:
* 91d6d87ca76e3aa27d5f87fd4f0b70f1d4fe4e72 replica install: fix thin 
client regression




Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.







Guys, I suspect that one of previously pushed patches breaks following 
command on the client side


# ipa dns-update-system-records
ipa: ERROR: KeyError: 'ipa_records'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1346, in run
sys.exit(api.Backend.cli.run(argv))
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line , in run
rv = cmd.output_for_cli(self.api.Backend.textui, result, *args, 
**options)
  File "/usr/lib/python2.7/site-packages/ipaclient/plugins/dns.py", 
line 367, in output_for_cli

textui.print_indented(u'{}:'.format(labels[key]), indent=1)
KeyError: 'ipa_records'
ipa: ERROR: an internal error has occurred

-- 
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] [WIP] Thin client

[Jan Cholasta]

On 20.6.2016 16:13, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.


Pushed to master: 8cc8b6fb1023fa4aeafac0df01cfacff4ebf537a

Note that I haven't pushed "replica install: fix thin client regression" 
yet, I would like Martin to review it first.


Attaching the patches for reference.



Note: There is one one known issue in automember output, will be fixed
later.



--
Jan Cholasta
From 82c9def5f01964a64e51ebf41b3bcfb4ab6a888c Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 3 Jun 2016 09:38:11 +0200
Subject: [PATCH 01/14] makeaci, makeapi: use in-server API

Capture the server API rather than client API in API.txt. Client API may be
affected by client-side plugins and thus may not correspond to what is
transmitted over the wire.

https://fedorahosted.org/freeipa/ticket/4739
---
 API.txt | 131 +++-
 makeaci |   4 +-
 makeapi |   4 +-
 3 files changed, 10 insertions(+), 129 deletions(-)

diff --git a/API.txt b/API.txt
index eb14d44..35ae5e4 100644
--- a/API.txt
+++ b/API.txt
@@ -343,13 +343,6 @@ output: Output('count', type=[])
 output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
-command: automountlocation_import
-args: 2,2,1
-arg: Str('cn', cli_name='location')
-arg: Str('masterfile')
-option: Flag('continue?', autofill=True, cli_name='continue', default=False)
-option: Str('version?')
-output: Output('result')
 command: automountlocation_show
 args: 1,4,3
 arg: Str('cn', cli_name='location')
@@ -761,7 +754,7 @@ option: Str('version?')
 output: Output('result')
 command: cert_request
 args: 1,6,1
-arg: File('csr', cli_name='csr_file')
+arg: Str('csr', cli_name='csr_file')
 option: Flag('add', autofill=True, default=False)
 option: Str('cacn?', cli_name='ca')
 option: Str('principal')
@@ -816,7 +809,7 @@ args: 1,6,3
 arg: Str('cn', cli_name='id')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('description', cli_name='desc')
-option: File('file', cli_name='file')
+option: Str('file', cli_name='file')
 option: Bool('ipacertprofilestoreissued', cli_name='store', default=True)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
@@ -830,7 +823,7 @@ option: Str('addattr*', cli_name='addattr')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('delattr*', cli_name='delattr')
 option: Str('description?', autofill=False, cli_name='desc')
-option: File('file?', cli_name='file')
+option: Str('file?', cli_name='file')
 option: Bool('ipacertprofilestoreissued?', autofill=False, cli_name='store', default=True)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Flag('rights', autofill=True, default=False)
@@ -3018,7 +3011,7 @@ arg: Str('ldapuri', cli_name='ldap_uri')
 arg: Password('bindpw', cli_name='password', confirm=False)
 option: DNParam('basedn?', cli_name='base_dn')
 option: DNParam('binddn?', autofill=True, cli_name='bind_dn', default=ipapython.dn.DN('cn=directory manager'))
-option: File('cacertfile?', cli_name='ca_cert_file')
+option: Str('cacertfile?', cli_name='ca_cert_file')
 option: Flag('compat?', autofill=True, cli_name='with_compat', default=False)
 option: Flag('continue?', autofill=True, default=False)
 option: Str('exclude_groups*', autofill=True, cli_name='exclude_groups', default=[])
@@ -3225,25 +3218,6 @@ option: Str('version?')
 output: Output('completed', type=[])
 output: Output('failed', type=[])
 output: Entry('result')
-command: otptoken_add_yubikey
-args: 1,13,3
-arg: Str('ipatokenuniqueid?', cli_name='id')
-option: Str('addattr*', cli_name='addattr')
-option: Flag('all', autofill=True, cli_name='all', default=False)
-option: Str('description?', cli_name='desc')
-option: Bool('ipatokendisabled?', cli_name='disabled')
-option: DateTime('ipatokennotafter?', 

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patches:
replica install: fix thin client regression
schema: remove `no_cli` from command schema
schema: remove redundant information
schema: merge command args and options
schema: remove output_params
schema: add object class schema
permission: handle ipapermright deprecated CLI alias on the client
passwd: handle sort order of passwd argument on the client
misc: skip `count` and `total` output in env.output_for_cli
dns: do not rely on custom param fields in record attributes
automember: add object plugin for automember_rebuild
frontend: do not crash on missing output in output_for_cli
frontend: skip `value` output in output_for_cli
frontend: don't copy command arguments to output params
makeaci, makeapi: use in-server API

work for me, ACK.

Note: There is one one known issue in automember output, will be fixed 
later.


--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 15.6.2016 13:56, David Kupka wrote:

On 06/15/2016 01:33 PM, David Kupka wrote:

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not
filled automatically. This will be fixed soon.

Otherwise works for me, ACK.



Fix for dynamic defaults regression is:

schema: fix client-side dynamic defaults

Works for me, ACK.


Pushed to master: d26e42ffb065cc524c63d65d87c2a2b5d943c54a

Attaching the patches for reference.

--
Jan Cholasta
From 108e818f772318b1dd0c6cc32f66750d0091b560 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 13:02:30 +0200
Subject: [PATCH 1/9] plugable: allow plugins to be non-classes

Allow registering any object that is callable and has `name` and `bases`
attributes as a plugin.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/plugable.py | 45 +++--
 ipalib/util.py | 26 ++
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 497b545..d8ff5c1 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -26,7 +26,6 @@ http://docs.python.org/ref/sequence-types.html
 """
 
 import sys
-import inspect
 import threading
 import os
 from os import path
@@ -40,6 +39,7 @@ import six
 from ipalib import errors
 from ipalib.config import Env
 from ipalib.text import _
+from ipalib.util import classproperty
 from ipalib.base import ReadOnly, NameSpace, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
 from ipapython.ipa_log_manager import (
@@ -101,8 +101,8 @@ class Registry(object):
 
 :param klass: A subclass of `Plugin` to attempt to register.
 """
-if not inspect.isclass(klass):
-raise TypeError('plugin must be a class; got %r' % klass)
+if not callable(klass):
+raise TypeError('plugin must be callable; got %r' % klass)
 
 # Raise DuplicateError if this exact class was already registered:
 if klass in self.__registry:
@@ -134,9 +134,18 @@ class Plugin(ReadOnly):
 self.__finalize_lock = threading.RLock()
 log_mgr.get_logger(self, True)
 
-@property
-def name(self):
-return type(self).__name__
+@classmethod
+def __name_getter(cls):
+return cls.__name__
+
+# you know nothing, pylint
+name = classproperty(__name_getter)
+
+@classmethod
+def __bases_getter(cls):
+return cls.__bases__
+
+bases = classproperty(__bases_getter)
 
 @property
 def doc(self):
@@ -571,12 +580,12 @@ class API(ReadOnly):
 :param klass: A subclass of `Plugin` to attempt to add.
 :param override: If true, override an already added plugin.
 """
-if not inspect.isclass(klass):
-raise TypeError('plugin must be a class; got %r' % klass)
+if not callable(klass):
+raise TypeError('plugin must be callable; got %r' % klass)
 
 # Find the base class or raise SubclassError:
-for base in self.bases:
-if issubclass(klass, self.bases):
+for base in klass.bases:
+if issubclass(base, self.bases):
 break
 else:
 raise errors.PluginSubclassError(
@@ -585,13 +594,13 @@ class API(ReadOnly):
 )
 
 # Check override:
-prev = self.__plugins.get(klass.__name__)
+prev = self.__plugins.get(klass.name)
 if prev:
 if not override:
 # Must use override=True to override:
 raise errors.PluginOverrideError(
 base=base.__name__,
-name=klass.__name__,
+name=klass.name,
 plugin=klass,
 )
 
@@ -601,12 +610,12 @@ class API(ReadOnly):
 # There was nothing already registered to override:
 raise errors.PluginMissingOverrideError(
 base=base.__name__,
-name=klass.__name__,
+name=klass.name,
 plugin=klass,
 )
 
 # The plugin is okay, add to sub_d:
-self.__plugins[klass.__name__] = klass
+self.__plugins[klass.name] = 

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 06/15/2016 01:33 PM, David Kupka wrote:

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not
filled automatically. This will be fixed soon.

Otherwise works for me, ACK.



Fix for dynamic defaults regression is:

schema: fix client-side dynamic defaults

Works for me, ACK.

--
David Kupka

--
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] [WIP] Thin client

[David Kupka]

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not 
filled automatically. This will be fixed soon.


Otherwise works for me, ACK.

--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 9.6.2016 08:14, David Kupka wrote:

On 08/06/16 14:44, Jan Cholasta wrote:

On 8.6.2016 14:40, Martin Babinsky wrote:

On 06/08/2016 02:11 PM, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries

This one is not complete since it breaks replica install w/ --setup-dns.
Removing this line seems to fix this case:

"""
diff --git a/ipaserver/install/server/replicainstall.py
b/ipaserver/install/server/replicainstall.py
index 41eee96..d695a15 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1478,7 +1478,6 @@ def promote(installer):
 server_api.finalize()

 if options.setup_dns:
-server_api.Backend.rpcclient.connect()
 server_api.Backend.ldap2.connect(autobind=True)
 dns.install(False, True, options, server_api
"""


Fixed.





schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and
I haven't met any new regression or bug, ACK.









Thanks for the catch, Martin. Now it's really fixed and can be pushed.
Obligatory keyword to trigger the push: ACK


Thanks. Attaching the patches for reference.

Pushed to master: 0f995312565e69768c660b85476b1efe1f62fb84



BTW, I've discovered other related issue [1] that is there since 4.2. I
will send patch soon.

[1] https://fedorahosted.org/freeipa/ticket/5945




--
Jan Cholasta
From 327c3e7309f3f18536eb964afdd25f7d211ad239 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 6 Jun 2016 12:14:21 +0200
Subject: [PATCH 1/5] schema: do not validate unrequested params in
 command_defaults

Request specific params when getting the defaults instead of getting
defaults for all params and filtering the result.

This fixes command_defaults failing with validation errors on unrequested
params.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/frontend.py  | 8 +---
 ipaserver/plugins/schema.py | 3 +--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 81e9cd4..ffcf71b 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -670,7 +670,7 @@ class Command(HasParam):
 if kw.get(param.name, None) is None:
 continue
 
-def get_default(self, **kw):
+def get_default(self, _params=None, **kw):
 """
 Return a dictionary of defaults for all missing required values.
 
@@ -687,8 +687,10 @@ class Command(HasParam):
 >>> c.get_default(color=u'Yellow')
 {}
 """
-params = [p.name for p in self.params() if p.name not in kw and (p.required or p.autofill)]
-return dict(self.__get_default_iter(params, kw))
+if _params is None:
+_params = [p.name for p in self.params()
+   if p.name not in kw and (p.required or p.autofill)]
+return dict(self.__get_default_iter(_params, kw))
 
 def get_default_of(self, _name, **kw):
 """
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index 8bc2303..d66943c 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -229,8 +229,7 @@ class command_defaults(PKQuery):
 raise errors.ConversionError(name=name,
  error=_("must be a dictionary"))
 
-result = command.get_default(**kw)
-result = {n: v for n, v in result.items() if n in params}
+result = command.get_default(params, **kw)
 
 return dict(result=result)
 
-- 
2.7.4

From 3ed6d04a80a6a7130481e23786eda19d0228bf15 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 6 Jun 2016 13:35:20 +0200
Subject: [PATCH 2/5] replica install: use remote server API to create service
 entries

Use the existing remote server API to create service entries instead of a
client API.

This fixes a crash during replica promotion due to unavailable schema.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/install/dsinstance.py|  6 +-
 ipaserver/install/installutils.py  | 23 ++--
 ipaserver/install/server/replicainstall.py | 90 +-
 ipaserver/plugins/service.py   |  2 +-
 4 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 00ef5f3..da0e743 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ 

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 08/06/16 14:44, Jan Cholasta wrote:

On 8.6.2016 14:40, Martin Babinsky wrote:

On 06/08/2016 02:11 PM, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries

This one is not complete since it breaks replica install w/ --setup-dns.
Removing this line seems to fix this case:

"""
diff --git a/ipaserver/install/server/replicainstall.py
b/ipaserver/install/server/replicainstall.py
index 41eee96..d695a15 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1478,7 +1478,6 @@ def promote(installer):
 server_api.finalize()

 if options.setup_dns:
-server_api.Backend.rpcclient.connect()
 server_api.Backend.ldap2.connect(autobind=True)
 dns.install(False, True, options, server_api
"""


Fixed.





schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and
I haven't met any new regression or bug, ACK.









Thanks for the catch, Martin. Now it's really fixed and can be pushed. 
Obligatory keyword to trigger the push: ACK


BTW, I've discovered other related issue [1] that is there since 4.2. I 
will send patch soon.


[1] https://fedorahosted.org/freeipa/ticket/5945

--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 8.6.2016 14:40, Martin Babinsky wrote:

On 06/08/2016 02:11 PM, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries

This one is not complete since it breaks replica install w/ --setup-dns.
Removing this line seems to fix this case:

"""
diff --git a/ipaserver/install/server/replicainstall.py
b/ipaserver/install/server/replicainstall.py
index 41eee96..d695a15 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1478,7 +1478,6 @@ def promote(installer):
 server_api.finalize()

 if options.setup_dns:
-server_api.Backend.rpcclient.connect()
 server_api.Backend.ldap2.connect(autobind=True)
 dns.install(False, True, options, server_api
"""


Fixed.





schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and
I haven't met any new regression or bug, ACK.







--
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] [WIP] Thin client

[Martin Babinsky]

On 06/08/2016 02:11 PM, David Kupka wrote:

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries
This one is not complete since it breaks replica install w/ --setup-dns. 
Removing this line seems to fix this case:


"""
diff --git a/ipaserver/install/server/replicainstall.py 
b/ipaserver/install/server/replicainstall.py

index 41eee96..d695a15 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1478,7 +1478,6 @@ def promote(installer):
 server_api.finalize()

 if options.setup_dns:
-server_api.Backend.rpcclient.connect()
 server_api.Backend.ldap2.connect(autobind=True)
 dns.install(False, True, options, server_api
"""



schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and
I haven't met any new regression or bug, ACK.




--
Martin^3 Babinsky

--
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] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries
schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and 
I haven't met any new regression or bug, ACK.


--
David Kupka

--
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] [WIP] Thin client

[David Kupka]

On 25/05/16 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump VERSION
for backward compatible changes after this whole patchset is merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be
pushed into master.
All upto "frontend: allow commands to have an argument named `name`" got
over numerous test cycles in last week+ and is working as expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Hello,
another patchset is ready. There are still some minor issues with 
interactive prompt in dns* commands but these can be fixed later. 
Otherwise all work as expected, ACK.


Also it would be good to have tests for schema plugin, I have filled a 
ticket [1].


List of commits in Honza's trac-4739 branch:
frontend: do not check API minor version of the client
ipalib: move server-side plugins to ipaserver
ipaclient: implement thin client
misc: hide the unused --all option of `env` and `plugins` in CLI
ipalib: move File command arguments to ipaclient
ipactl: use server API
client install: finalize API after CA certs are available
rpc: do not validate command name in RPCClient.forward
rpc: optimize JSON-RPC response handling
rpc: specify connection options in API config
rpc: allow overriding NSS DB directory in API config
rpc: respect API config in RPCClient.create_connection
ipalib: introduce API schema plugins
ipalib: replace DeprecatedParam with `deprecated` Param argument
parameters: introduce no_convert keyword argument
parameters: introduce cli_metavar keyword argument
ipalib: split off client-side plugin code into ipaclient
dns: move code shared by client and server to separate module
ipaclient: add client-side command override class
frontend: turn Method attributes into properties
plugable: remember overriden plugins in API
plugable: 

Re: [Freeipa-devel] [WIP] Thin client

[Pavel Vomacka]

On 05/30/2016 08:28 AM, Jan Cholasta wrote:

On 28.5.2016 18:33, Pavel Vomacka wrote:



On 05/27/2016 10:33 AM, Petr Spacek wrote:

On 27.5.2016 09:26, Martin Basti wrote:


On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:


On 26.05.2016 11:21, Martin Basti wrote:


On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:


On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:


On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:


On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for 
cross-plugin

imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code 
called on

client *
1)
you forgot to increment VERSION
This was deliberate, as it will no longer be necessary to 
bump

VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

Fixed.


2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an 
error if

omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?

The command definition was not complete, it was missing
has_output.


Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version

Fixed.


* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this 
change is

needed
in commit message

Fixed.


The other commits I will check later.
Martin^2


Okay. Thanks.


* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, 
but I

havent
found anything better to use.

It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is 
mainly

copy of
current to proper places, LGTM

It's actually mostly cut-n-paste.


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

The next change would be impossible without this.

* plugable: replace API.import_plugins with new 
API.add_package

LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in 
docstrings

LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to 
Command

 LGTM

Martin^2





Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is 
ready

to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.

Thanks for the review.


Honzo, please rebase and push them, thanks!


Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055


Guys, you forgot to test it with newer pylint.

I don't see any "BuildRequires: newer pylint" in the spec file.


Patch attached.
LGTM, although the commit message is wrong - this is not 
related to
thin client at all, PublicError and PublicMessage had .kw since 
forever.



updated commit message






Can I push that fix for pylint?

Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.
Yes, 'data' is a dict which contains structured message data. I 
did not see
these specific failures before, though. Just add it to expected 
results

where missing.

E   AssertionError: assert_deepequal: dict keys 
mismatch.
E 0026: permission_find: Search for u'testperm' 
with a

limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 28.5.2016 18:33, Pavel Vomacka wrote:



On 05/27/2016 10:33 AM, Petr Spacek wrote:

On 27.5.2016 09:26, Martin Basti wrote:


On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:


On 26.05.2016 11:21, Martin Basti wrote:


On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:


On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:


On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:


On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION

This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

Fixed.


2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?

The command definition was not complete, it was missing
has_output.


Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version

Fixed.


* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message

Fixed.


The other commits I will check later.
Martin^2


Okay. Thanks.


* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.

It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM

It's actually mostly cut-n-paste.


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

The next change would be impossible without this.


* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2





Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.

Thanks for the review.


Honzo, please rebase and push them, thanks!


Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055


Guys, you forgot to test it with newer pylint.

I don't see any "BuildRequires: newer pylint" in the spec file.


Patch attached.

LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since forever.


updated commit message






Can I push that fix for pylint?

Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.

Yes, 'data' is a dict which contains structured message data. I did not see
these specific failures before, though. Just add it to expected results
where missing.


E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size 

Re: [Freeipa-devel] [WIP] Thin client

[Pavel Vomacka]

On 05/27/2016 10:33 AM, Petr Spacek wrote:

On 27.5.2016 09:26, Martin Basti wrote:


On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:


On 26.05.2016 11:21, Martin Basti wrote:


On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:


On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:


On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:


On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION

This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

Fixed.


2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
  -del answer['summary']  # Why does this cause an
error if
omitted?

The command definition was not complete, it was missing
has_output.


Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version

Fixed.


* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message

Fixed.


The other commits I will check later.
Martin^2


Okay. Thanks.


* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.

It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM

It's actually mostly cut-n-paste.


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

The next change would be impossible without this.


* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
  LGTM

Martin^2





Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.

Thanks for the review.


Honzo, please rebase and push them, thanks!


Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055


Guys, you forgot to test it with newer pylint.

I don't see any "BuildRequires: newer pylint" in the spec file.


Patch attached.

LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since forever.


updated commit message






Can I push that fix for pylint?

Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.

Yes, 'data' is a dict which contains structured message data. I did not see
these specific failures before, though. Just add it to expected results
where missing.


E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size limit exceeded', 'code': 13017, 'type':

Re: [Freeipa-devel] [WIP] Thin client

[Petr Spacek]

On 27.5.2016 09:26, Martin Basti wrote:
> 
> 
> On 27.05.2016 07:49, Jan Cholasta wrote:
>> On 26.5.2016 18:43, Martin Basti wrote:
>>>
>>>
>>> On 26.05.2016 11:21, Martin Basti wrote:


 On 26.05.2016 07:15, Jan Cholasta wrote:
> On 25.5.2016 18:17, Martin Basti wrote:
>>
>>
>> On 25.05.2016 16:07, Jan Cholasta wrote:
>>> On 25.5.2016 15:03, David Kupka wrote:
 On 18/05/16 08:01, Jan Cholasta wrote:
> On 16.5.2016 16:35, Martin Basti wrote:
>>
>>
>> On 09.05.2016 14:07, Jan Cholasta wrote:
>>> On 6.5.2016 14:32, Martin Basti wrote:


 On 28.04.2016 14:45, Jan Cholasta wrote:
> Hi,
>
> I have pushed my thin client WIP branch to GitHub:
> .
>
> All commits up to "ipalib: use relative imports for cross-plugin
> imports" should be good for review. The rest is subject to
> change
> (WARNING: I will force push into this branch).
>
> Honza
>
 I did not test it yet, I just checked the code

 * automount: do not inherit automountlocation_tofiles from
 LDAPQuery *
 LGTM

 * dns: move all dnsrecord code called on client to a single
 class *
 LGTM

 * dns: do not rely on server data structures in code called on
 client *
 1)
 you forgot to increment VERSION
>>>
>>> This was deliberate, as it will no longer be necessary to bump
>>> VERSION
>>> for backward compatible changes after this whole patchset is
>>> merged.
>>> But we're not there yet, so fixed.
>>>
>> How we should handle VERSION after your patches?

 Otherwise LGTM

 * otptoken: fix import of DN *
 ACK

 * otptoken_yubikey: fix otptoken_add_yubikey arguments *
 1)
 you forgot to increment VERSION
>>>
>>> Fixed.
>>>

 2)
 Did you find out why this was issue?
 -del answer['value']# Why does this cause an error if
 omitted?
  -del answer['summary']  # Why does this cause an
 error if
 omitted?
>>>
>>> The command definition was not complete, it was missing
>>> has_output.
>>>

 Otherwise LGTM

 * vault: move client-side code to the module level *
 LGTM

 * vault: copy arguments of client commands from server
 counterparts *
 1)
 you forgot to increment Version
>>>
>>> Fixed.
>>>

 * ipalib: use relative imports for cross-plugin imports *
 1) Missing explanation for future generations why this change is
 needed
 in commit message
>>>
>>> Fixed.
>>>

 The other commits I will check later.
 Martin^2

>>>
>>> Okay. Thanks.
>>>
>>
>> * frontend: remove the unused Command.soft_validate method
>> LGTM
>>
>> * frontend: perform argument value validation only on server
>> LGTM
>>
>> * frontend: do not forward argument defaults to server
>> I'm not a fan of returning  None in promt_param function, but I
>> havent
>> found anything better to use.
>
> It always returned None for unset params.
>
>> LGTM
>>
>> * ipalib: optimize API.txt
>> this contains a lot of black magic, but because this is mainly
>> copy of
>> current to proper places, LGTM
>
> It's actually mostly cut-n-paste.
>
>>
>> * makeaci: load additional plugins using API.add_module
>> Looks good, but I miss explanation why is this change needed
>
> The next change would be impossible without this.
>
>>
>> * plugable: replace API.import_plugins with new API.add_package
>> LGTM
>>
>>
>> * ipalib, ipaserver: migrate all plugins to Registry-based
>> registration
>> ACK
>>
>> * ipalib, ipaserver: fix incorrect API.register calls in docstrings
>> LGTM
>>
>>
>> * plugable: remove the unused deprecated API.register method
>> LGTM
>>
>>
>> * plugable: switch API to Registry-based plugin discovery
>> LGTM
>>
>> * frontend: 

Re: [Freeipa-devel] [WIP] Thin client

[Martin Basti]

On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:



On 26.05.2016 11:21, Martin Basti wrote:



On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for 
cross-plugin

imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an 
error if

omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?


The command definition was not complete, it was missing
has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this 
change is

needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in 
docstrings

LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since 
forever.



updated commit message






Can I push that fix for pylint?


Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b






I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.


Yes, 'data' is a dict which contains structured message data. I did 
not see these specific failures before, though. Just add it to 
expected results where missing.




E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size limit exceeded', 'code': 13017, 'type':
u'warning', 'name': 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 26.5.2016 18:43, Martin Basti wrote:



On 26.05.2016 11:21, Martin Basti wrote:



On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?


The command definition was not complete, it was missing
has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since forever.


updated commit message






Can I push that fix for pylint?


Sure, ACK.



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.


Yes, 'data' is a dict which contains structured message data. I did not 
see these specific failures before, though. Just add it to expected 
results where missing.




E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size limit exceeded', 'code': 13017, 'type':
u'warning', 'name': u'SearchResultTruncated'}
E got = {u'data': {u'reason': u'Configured size limit
exceeded'}, u'message': u'Search result has 

Re: [Freeipa-devel] [WIP] Thin client

[Martin Basti]

On 26.05.2016 11:21, Martin Basti wrote:



On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to 
change

(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single 
class *

LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is 
merged.

But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an 
error if

omitted?


The command definition was not complete, it was missing 
has_output.




Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I 
havent

found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly 
copy of

current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready 
to be

pushed into master.
All upto "frontend: allow commands to have an argument named 
`name`" got
over numerous test cycles in last week+ and is working as 
expected

now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to 
thin client at all, PublicError and PublicMessage had .kw since forever.



updated commit message






Can I push that fix for pylint?

I found something suspicious in tests, did anything in IPA messages 
change? I suspect that this is related to client_patches.


E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a 
limit of 1 (truncated) with members

E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been 
truncated: Configured size limit exceeded', 'code': 13017, 'type': 
u'warning', 'name': u'SearchResultTruncated'}
E got = {u'data': {u'reason': u'Configured size limit 
exceeded'}, u'message': u'Search result has been truncated: Configured 
size limit exceeded', u'code': 13017, u'type': u'warning', u'name': 
u'SearchResultTruncated'}

E path = ('messages', 0)



E   

Re: [Freeipa-devel] [WIP] Thin client

[Martin Basti]

On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single 
class *

LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is 
merged.

But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I 
havent

found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly 
copy of

current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be
pushed into master.
All upto "frontend: allow commands to have an argument named 
`name`" got

over numerous test cycles in last week+ and is working as expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to 
thin client at all, PublicError and PublicMessage had .kw since forever.



updated commit message



From a8a759a9f64ada4defda8cb3bc9021d93e3e64ce Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 25 May 2016 18:12:57 +0200
Subject: [PATCH] fix pylint false positive errors

pylint 1.5 reports 'kw' as 'no-member' for PublicError and
PublicMessage. It is false positive in both cases.

https://fedorahosted.org/freeipa/ticket/4739
---
 pylint_plugins.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pylint_plugins.py b/pylint_plugins.py
index 7460d9f4a4c45fd036a32da3e746a4836791a0c2..631d85ab1d9e79b7b90cd1c7742e64a5b74e4f61 100644
--- a/pylint_plugins.py
+++ b/pylint_plugins.py
@@ -114,6 +114,7 @@ ipa_class_members = {
 'ipalib.errors.PublicError': [
 'msg',
 'strerror',
+'kw',
 ],
 'ipalib.errors.SingleMatchExpected': [
 'found',
@@ -128,6 +129,7 @@ ipa_class_members = {
 'msg',
 'strerror',
 'type',
+'kw',
 ],
 'ipalib.parameters.Param': [
 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 25.5.2016 18:17, Martin Basti wrote:



On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be
pushed into master.
All upto "frontend: allow commands to have an argument named `name`" got
over numerous test cycles in last week+ and is working as expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.


I don't see any "BuildRequires: newer pylint" in the spec file.



Patch attached.


LGTM, although the commit message is wrong - this is not related to thin 
client at all, PublicError and PublicMessage had .kw since forever.


--
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] [WIP] Thin client

[Martin Basti]

On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from 
LDAPQuery *

LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump 
VERSION

for backward compatible changes after this whole patchset is merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server 
counterparts *

1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based 
registration

ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be
pushed into master.
All upto "frontend: allow commands to have an argument named `name`" got
over numerous test cycles in last week+ and is working as expected
now, ACK.


Thanks for the review.



Honzo, please rebase and push them, thanks!



Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055



Guys, you forgot to test it with newer pylint.

Patch attached.
From 5714b4b1dcad5618f2dccd79dcd5022ea859ed63 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 25 May 2016 18:12:57 +0200
Subject: [PATCH] fix pylint false positive errors related to thin client

pylint 1.5 reports 'kw' as 'no-member' for PublicError and
PublicMessage. It is false positive in both cases.

https://fedorahosted.org/freeipa/ticket/4739
---
 pylint_plugins.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pylint_plugins.py b/pylint_plugins.py
index 7460d9f4a4c45fd036a32da3e746a4836791a0c2..631d85ab1d9e79b7b90cd1c7742e64a5b74e4f61 100644
--- a/pylint_plugins.py
+++ b/pylint_plugins.py
@@ -114,6 +114,7 @@ ipa_class_members = {
 'ipalib.errors.PublicError': [
 'msg',
 'strerror',
+'kw',
 ],
 'ipalib.errors.SingleMatchExpected': [
 'found',
@@ -128,6 +129,7 @@ ipa_class_members = {
 'msg',
 'strerror',
 'type',
+'kw',
 ],
 'ipalib.parameters.Param': [
 'cli_name',
-- 
2.5.5

-- 
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] [WIP] Thin client

[David Kupka]

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump VERSION
for backward compatible changes after this whole patchset is merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2







Hello,
first batch of 30 patches from Honza's trac-4739 branch 
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready to be 
pushed into master.
All upto "frontend: allow commands to have an argument named `name`" got 
over numerous test cycles in last week+ and is working as expected 
now, ACK.


Honzo, please rebase and push them, thanks!

--
David Kupka

--
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] [WIP] Thin client

[Jan Cholasta]

On 16.5.2016 16:35, Martin Basti wrote:



On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump VERSION
for backward compatible changes after this whole patchset is merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an error if
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent
found anything better to use.


It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of
current to proper places, LGTM


It's actually mostly cut-n-paste.



* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed


The next change would be impossible without this.



* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2





--
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] [WIP] Thin client

[David Kupka]

On 28/04/16 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hi,
I have fought my way through the patch set up to "frontend: allow 
commands to have an argument named `name`" (including).
The code looks good to me but I haven't dive into parts that was pure 
Cut I assume that when it worked before it will still work.


Some problems pop out in tests:

user_plugin: http://paste.fedoraproject.org/367502/34888591/
This can be fixed with simple patch: 
http://paste.fedoraproject.org/367503/14634889/


With patch applied there are still some errors: 
http://paste.fedoraproject.org/367519/49007414/


--
David Kupka

--
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] [WIP] Thin client

[Martin Basti]

On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump VERSION 
for backward compatible changes after this whole patchset is merged. 
But we're not there yet, so fixed.



How we should handle VERSION after your patches?


Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if 
omitted?
 -del answer['summary']  # Why does this cause an error if 
omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.



* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I havent 
found anything better to use.

LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly copy of 
current to proper places, LGTM


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2


--
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] [WIP] Thin client

[Jan Cholasta]

On 6.5.2016 14:32, Martin Basti wrote:



On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on client *
1)
you forgot to increment VERSION


This was deliberate, as it will no longer be necessary to bump VERSION 
for backward compatible changes after this whole patchset is merged. But 
we're not there yet, so fixed.




Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION


Fixed.



2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if omitted?
 -del answer['summary']  # Why does this cause an error if omitted?


The command definition was not complete, it was missing has_output.



Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version


Fixed.



* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is needed
in commit message


Fixed.



The other commits I will check later.
Martin^2



Okay. Thanks.

--
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] [WIP] Thin client

[Martin Basti]

On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub: 
.


All commits up to "ipalib: use relative imports for cross-plugin 
imports" should be good for review. The rest is subject to change 
(WARNING: I will force push into this branch).


Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single class *
LGTM

* dns: do not rely on server data structures in code called on client *
1)
you forgot to increment VERSION

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if omitted?
 -del answer['summary']  # Why does this cause an error if omitted?

Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server counterparts *
1)
you forgot to increment Version

* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is needed 
in commit message


The other commits I will check later.
Martin^2

--
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] [WIP] Thin client

[Jan Cholasta]

Hi,

I have pushed my thin client WIP branch to GitHub: 
.


All commits up to "ipalib: use relative imports for cross-plugin 
imports" should be good for review. The rest is subject to change 
(WARNING: I will force push into this branch).


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