Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-02-10 Thread Jan Zeleny
Jan Zelený jzel...@redhat.com wrote:
 Ok, I'm sending updated patch in attachment
 
   Should I change it in class help then? That's where I copied this from.
  
  I think so.
 
 Ok, I'll send another patch, so me don't mix it together with this patch.
 I'll do a review of the code in cli.py, maybe the same issue is elsewhere
 as well.
 
   This will blow up as expected in the FIXME if an unknown command is
   passed in.
   
   Fixed, thanks.
  
  Not to be pedantic but I think it should return a non-zero error code
  too on error.
 
 Yep, replaced this with exception.
 
   ipa show-mappings user-show returns just 'rights'
   
   If it was the acting correctly, it shouldn't be displayed at all,
   because it is not LDAP based (and user-show doesn't take any other
   LDAP-based arguments/options).
   
   I'm just not sure how to do this with minimal changes. One option is to
   create new flag denoting whether parameter is LDAP based or not and for
   each parameter set it appropriately, but that is just too much effort
   for something that is not that important. That's why I use the 'webui'
   flag to filter things at least a little bit.
  
  You should have the object Params list available, right? Can you use
  that to show at least some attributes?
 
 I already thought of that, but that would add only primary key, since
 Params is a concatenation of Options and Args - in args there are usually
 only mandatory arguments (i.e. primary keys, uid in case of user-show) and
 options are already iterated over and printed out.
 
 I think adding this is too much effort. For one thing user-show takes no
 other options than --rights (and the purpose of the patch is to show
 mapping between CLI options and LDAP attributes) and user can always see
 real LDAP attributes of user object by using --raw.
 
 Jan

Just a reminder that this patch waits for review.

Thanks
Jan

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


Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-02-10 Thread Jan Zeleny
Rob Crittenden rcrit...@redhat.com wrote:
 Just a really minor nit. Can you define a label for the argument?
 Otherwise if you run: `ipa show-mappings` it will prompt for
 command_name.
 
 rob

Done, sending in attachment.

Jan
From fece796ab7894a591ef4e2fb4bb39f097c687cc1 Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Wed, 26 Jan 2011 13:09:26 +0100
Subject: [PATCH] Provide a way to display CLI-LDAP relation

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

https://fedorahosted.org/freeipa/ticket/447
---
 ipalib/cli.py |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 4ce7e7fa91c347aa629a8bf9dd5964f4120fb539..ab26dba69ff122637f7bcadbc71c44c9ccf76b64 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -47,7 +47,7 @@ import plugable
 import util
 from errors import PublicError, CommandError, HelpError, InternalError, NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError
 from constants import CLI_TAB
-from parameters import Password, Bytes, File
+from parameters import Password, Bytes, File, Str
 from text import _
 from ipapython.version import API_VERSION
 
@@ -767,6 +767,30 @@ class help(frontend.Local):
 print '  %s  %s' % (to_cli(c.name).ljust(mcl), c.summary)
 print \n
 
+class show_mappings(frontend.Command):
+takes_args = (
+Str('command_name',
+label='Command name',
+),
+)
+has_output = tuple()
+
+def run(self, command_name):
+command_name = from_cli(command_name)
+if command_name not in self.Command:
+raise CommandError(name=command_name)
+params = self.Command[command_name].options
+out = [('Parameter','LDAP attribute'),
+   ('=','==')]
+mcl = len(out[0][0])
+for param in params():
+if param.exclude and 'webui' in param.exclude:
+continue
+out.append((param.cli_name, param.param_spec))
+mcl = max(mcl,len(param.cli_name))
+for item in out:
+print to_cli(item[0]).ljust(mcl)+' : '+item[1]
+
 
 class console(frontend.Command):
 Start the IPA interactive Python console.
@@ -1045,6 +1069,7 @@ cli_plugins = (
 textui,
 console,
 help,
+show_mappings,
 )
 
 
-- 
1.7.1

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

Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-02-10 Thread Rob Crittenden

Jan Zeleny wrote:

Rob Crittendenrcrit...@redhat.com  wrote:

Just a really minor nit. Can you define a label for the argument?
Otherwise if you run: `ipa show-mappings` it will prompt for
command_name.

rob


Done, sending in attachment.

Jan


I made one minor change to the patch before pushing. I wrapper 'Command 
name' in _() so it can be localized.


pushed to master

rob

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


Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-02-01 Thread Rob Crittenden

Jan Zelený wrote:

Rob Crittendenrcrit...@redhat.com  wrote:

Jan Zelený wrote:

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

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

When reviewing, please pay attention to line 39 of the patch (detection
of the 'webui' in param.excludes). I think this is the right approach,
but I'm not 100% sure.

Thanks
Jan


nack.


I'm sending updated patch. Few comments:


The argument should be a Str, not Bytes.


Should I change it in class help then? That's where I copied this from.


I think so.



This will blow up as expected in the FIXME if an unknown command is
passed in.


Fixed, thanks.


Not to be pedantic but I think it should return a non-zero error code 
too on error.





ipa show-mappings user-show returns just 'rights'


If it was the acting correctly, it shouldn't be displayed at all, because it
is not LDAP based (and user-show doesn't take any other LDAP-based
arguments/options).

I'm just not sure how to do this with minimal changes. One option is to create
new flag denoting whether parameter is LDAP based or not and for each parameter
set it appropriately, but that is just too much effort for something that is
not that important. That's why I use the 'webui' flag to filter things at least
a little bit.


You should have the object Params list available, right? Can you use 
that to show at least some attributes?





Should it take a second arg or an option to lookup a specific
attribute/option pair?


Frankly I don't see any real benefit. I thought about it when Dmitri suggested
it, but commands don't take that many options - IMO it's not a problem to find
one in a list of ten.


Ok, that's true

rob

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


Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-02-01 Thread Jan Zelený
Ok, I'm sending updated patch in attachment

  Should I change it in class help then? That's where I copied this from.
 
 I think so.

Ok, I'll send another patch, so me don't mix it together with this patch. I'll 
do a review of the code in cli.py, maybe the same issue is elsewhere as well.

  This will blow up as expected in the FIXME if an unknown command is
  passed in.
  
  Fixed, thanks.
 
 Not to be pedantic but I think it should return a non-zero error code
 too on error.

Yep, replaced this with exception.

  ipa show-mappings user-show returns just 'rights'
  
  If it was the acting correctly, it shouldn't be displayed at all, because
  it is not LDAP based (and user-show doesn't take any other LDAP-based
  arguments/options).
  
  I'm just not sure how to do this with minimal changes. One option is to
  create new flag denoting whether parameter is LDAP based or not and for
  each parameter set it appropriately, but that is just too much effort
  for something that is not that important. That's why I use the 'webui'
  flag to filter things at least a little bit.
 
 You should have the object Params list available, right? Can you use
 that to show at least some attributes?

I already thought of that, but that would add only primary key, since Params 
is a concatenation of Options and Args - in args there are usually only 
mandatory arguments (i.e. primary keys, uid in case of user-show) and options 
are already iterated over and printed out.

I think adding this is too much effort. For one thing user-show takes no other 
options than --rights (and the purpose of the patch is to show mapping between 
CLI options and LDAP attributes) and user can always see real LDAP attributes 
of user object by using --raw.

Jan
From 5abec649ec6d9bfc82bc29290961cbcf9e8c94cb Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Wed, 26 Jan 2011 13:09:26 +0100
Subject: [PATCH] Provide a way to display CLI-LDAP relation

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

https://fedorahosted.org/freeipa/ticket/447
---
 ipalib/cli.py |   23 ++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 5543301c0c9039dc67b159c06526a0bdb3581c88..fa8d36d356076103c3310b7a30f5834ec23350bd 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -47,7 +47,7 @@ import plugable
 import util
 from errors import PublicError, CommandError, HelpError, InternalError, NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError
 from constants import CLI_TAB
-from parameters import Password, Bytes, File
+from parameters import Password, Bytes, File, Str
 from text import _
 from ipapython.version import API_VERSION
 
@@ -779,6 +779,26 @@ class help(frontend.Local):
 print '  %s  %s' % (to_cli(c.name).ljust(mcl), c.summary)
 print \n
 
+class show_mappings(frontend.Command):
+takes_args = (Str('command_name'),)
+has_output = tuple()
+
+def run(self, command_name):
+command_name = from_cli(command_name)
+if command_name not in self.Command:
+raise CommandError(name=command_name)
+params = self.Command[command_name].options
+out = [('Parameter','LDAP attribute'),
+   ('=','==')]
+mcl = len(out[0][0])
+for param in params():
+if param.exclude and 'webui' in param.exclude:
+continue
+out.append((param.cli_name, param.param_spec))
+mcl = max(mcl,len(param.cli_name))
+for item in out:
+print to_cli(item[0]).ljust(mcl)+' : '+item[1]
+
 
 class console(frontend.Command):
 Start the IPA interactive Python console.
@@ -1047,6 +1067,7 @@ cli_plugins = (
 textui,
 console,
 help,
+show_mappings,
 )
 
 
-- 
1.7.3.4

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

Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-01-31 Thread Jan Zelený
Rob Crittenden rcrit...@redhat.com wrote:
 Jan Zelený wrote:
  Since some LDAP attributes have their cli_name value defined,
  so they can be more user friendly, it can be difficult for user to find
  out which attributes do the parameteres given to CLI really represent.
  This patch provides new command, which will take another IPA command as
  and argument and display attributes which given command takes and what
  LDAP attributes are they mapped to.
  
  https://fedorahosted.org/freeipa/ticket/447
  
  When reviewing, please pay attention to line 39 of the patch (detection
  of the 'webui' in param.excludes). I think this is the right approach,
  but I'm not 100% sure.
  
  Thanks
  Jan
 
 nack.

I'm sending updated patch. Few comments:

 The argument should be a Str, not Bytes.

Should I change it in class help then? That's where I copied this from.
 
 This will blow up as expected in the FIXME if an unknown command is
 passed in.

Fixed, thanks.

 ipa show-mappings user-show returns just 'rights'

If it was the acting correctly, it shouldn't be displayed at all, because it 
is not LDAP based (and user-show doesn't take any other LDAP-based 
arguments/options).

I'm just not sure how to do this with minimal changes. One option is to create 
new flag denoting whether parameter is LDAP based or not and for each parameter 
set it appropriately, but that is just too much effort for something that is 
not that important. That's why I use the 'webui' flag to filter things at least 
a little bit.

 Should it take a second arg or an option to lookup a specific
 attribute/option pair?

Frankly I don't see any real benefit. I thought about it when Dmitri suggested 
it, but commands don't take that many options - IMO it's not a problem to find 
one in a list of ten.

Jan
From 2db9043eb65006890b7e500502647a5a4d8e3fb5 Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Wed, 26 Jan 2011 13:09:26 +0100
Subject: [PATCH] Provide a way to display CLI-LDAP relation

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

https://fedorahosted.org/freeipa/ticket/447
---
 ipalib/cli.py |   24 +++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 5543301c0c9039dc67b159c06526a0bdb3581c88..411abc6ba6792da1791b0d9450f709ce6cedfd4d 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -47,7 +47,7 @@ import plugable
 import util
 from errors import PublicError, CommandError, HelpError, InternalError, NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError
 from constants import CLI_TAB
-from parameters import Password, Bytes, File
+from parameters import Password, Bytes, File, Str
 from text import _
 from ipapython.version import API_VERSION
 
@@ -779,6 +779,27 @@ class help(frontend.Local):
 print '  %s  %s' % (to_cli(c.name).ljust(mcl), c.summary)
 print \n
 
+class show_mappings(frontend.Command):
+takes_args = (Str('command_name'),)
+has_output = tuple()
+
+def run(self, command_name):
+command_name = from_cli(command_name)
+if command_name not in self.Command:
+print 'ERROR: unknown command \''+command_name+'\''
+return
+params = self.Command[command_name].options
+out = [('Parameter','LDAP attribute'),
+   ('=','==')]
+mcl = len(out[0][0])
+for param in params():
+if param.exclude and 'webui' in param.exclude:
+continue
+out.append((param.cli_name, param.param_spec))
+mcl = max(mcl,len(param.cli_name))
+for item in out:
+print to_cli(item[0]).ljust(mcl)+' : '+item[1]
+
 
 class console(frontend.Command):
 Start the IPA interactive Python console.
@@ -1047,6 +1068,7 @@ cli_plugins = (
 textui,
 console,
 help,
+show_mappings,
 )
 
 
-- 
1.7.3.4

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

Re: [Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-01-28 Thread Rob Crittenden

Jan Zelený wrote:

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

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

When reviewing, please pay attention to line 39 of the patch (detection of the
'webui' in param.excludes). I think this is the right approach, but I'm not
100% sure.

Thanks
Jan


nack.

The argument should be a Str, not Bytes.

This will blow up as expected in the FIXME if an unknown command is 
passed in.


ipa show-mappings user-show returns just 'rights'

Should it take a second arg or an option to lookup a specific 
attribute/option pair?


rob

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


[Freeipa-devel] [PATCH] Provide a way to display CLI-LDAP attribute relation

2011-01-26 Thread Jan Zelený
Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

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

When reviewing, please pay attention to line 39 of the patch (detection of the 
'webui' in param.excludes). I think this is the right approach, but I'm not 
100% sure.

Thanks
Jan
From 6021801d788893ae467facd79379a8dfcf5842f7 Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Wed, 26 Jan 2011 13:09:26 +0100
Subject: [PATCH] Provide a way to display CLI-LDAP relation

Since some LDAP attributes have their cli_name value defined,
so they can be more user friendly, it can be difficult for user to find
out which attributes do the parameteres given to CLI really represent.
This patch provides new command, which will take another IPA command as
and argument and display attributes which given command takes and what
LDAP attributes are they mapped to.

https://fedorahosted.org/freeipa/ticket/447
---
 ipalib/cli.py |   21 +
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 2d219b71bd1a17b0dc2977b3b7048d8318203c63..c2e8bdcf646d7f2c1a06de5053c1c82ee61695af 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -786,6 +786,26 @@ class help(frontend.Local):
 print '  %s  %s' % (to_cli(c.name).ljust(mcl), c.summary)
 print \n
 
+class show_mappings(frontend.Command):
+takes_args = (Bytes('command_name'),)
+has_output = tuple()
+
+def run(self, command_name):
+command_name = from_cli(command_name)
+if command_name not in self.Command:
+print 'error: TODO fill in the text'
+params = self.Command[command_name].options
+out = [('Parameter','LDAP attribute'),
+   ('=','==')]
+mcl = len(out[0][0])
+for param in params():
+if param.exclude and 'webui' in param.exclude:
+continue
+out.append((param.cli_name, param.param_spec))
+mcl = max(mcl,len(param.cli_name))
+for item in out:
+print to_cli(item[0]).ljust(mcl)+' : '+item[1]
+
 
 class console(frontend.Command):
 Start the IPA interactive Python console.
@@ -1054,6 +1074,7 @@ cli_plugins = (
 textui,
 console,
 help,
+show_mappings,
 )
 
 
-- 
1.7.3.4

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