Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right

2014-03-21 Thread Petr Viktorin 

On 03/20/2014 07:20 PM, Misnyovszki Adam wrote:

On Tue, 18 Mar 2014 12:02:06 +0100
Petr Viktorin pvikt...@redhat.com wrote:


Hello,
This renames --permissions to --right. The old name is kept as a
deprecated alias.
FreeIPA didn't have a mechanism for doing this, so I added one.
Also, while I was digging around in this part, I made the new IntEnum
(and all future Enums) act like StrEnum in --help output.


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



499 ACK
500 ACK
501 ACK
  - although should it allow mixing deprecated and current aliases(eg
--permission=read --right=write)?


You're right, this is a strange edge case, but detecting this would need 
need a much more complicated approach than sharing the option's `dest`. 
I don't think it's worth it.



  - works fine with cli / webui also
  - help displays nicely
502
  - tested with more than one deprecated alias - API.txt validation
doesn't match, although it has the same output:
Got StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias', 'permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))
Expected StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))

API.txt:
option: StrEnum('ipapermright', attribute=True,
cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))
ipalib/plugins/permission.py:
 StrEnum(
 'ipapermright*',
 cli_name='right',
 deprecated_cli_aliases={'permissions','testalias'},
 label=_('Granted
rights'),
 doc=_('Rights to grant
'
   '(read, search, compare, write, add, delete,
all)'),
 values=(u'read', u'search',
u'compare',
 u'write', u'add', u'delete',
u'all'),
 ),
don't know if it is a problem anyways
  - other tests(cli, webui) works fine for me
  - unit tests related to this ran as expected
so besides the multiple deprecated_cli_aliases issue, it's an ACK


It looks like you've edited API.txt by hand and forgot a space after the 
comma in ['testalias','permissions']. Does it work if you use makeapi to 
regenerate API.txt?


--
Petr³

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

Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right

2014-03-21 Thread Misnyovszki Adam 
On Fri, 21 Mar 2014 11:14:43 +0100
Petr Viktorin pvikt...@redhat.com wrote:

 On 03/20/2014 07:20 PM, Misnyovszki Adam wrote:
  On Tue, 18 Mar 2014 12:02:06 +0100
  Petr Viktorin pvikt...@redhat.com wrote:
 
  Hello,
  This renames --permissions to --right. The old name is kept as a
  deprecated alias.
  FreeIPA didn't have a mechanism for doing this, so I added one.
  Also, while I was digging around in this part, I made the new
  IntEnum (and all future Enums) act like StrEnum in --help output.
 
 
  https://fedorahosted.org/freeipa/ticket/4231
 
 
  499 ACK
  500 ACK
  501 ACK
- although should it allow mixing deprecated and current
  aliases(eg --permission=read --right=write)?
 
 You're right, this is a strange edge case, but detecting this would
 need need a much more complicated approach than sharing the option's
 `dest`. I don't think it's worth it.
 
- works fine with cli / webui also
- help displays nicely
  502
- tested with more than one deprecated alias - API.txt validation
  doesn't match, although it has the same output:
  Got StrEnum('ipapermright', attribute=True, cli_name='right',
  deprecated_cli_aliases=set(['testalias', 'permissions']),
  multivalue=True, required=False, values=(u'read', u'search',
  u'compare', u'write', u'add', u'delete', u'all'))
  Expected StrEnum('ipapermright', attribute=True, cli_name='right',
  deprecated_cli_aliases=set(['testalias','permissions']),
  multivalue=True, required=False, values=(u'read', u'search',
  u'compare', u'write', u'add', u'delete', u'all'))
 
  API.txt:
  option: StrEnum('ipapermright', attribute=True,
  cli_name='right',
  deprecated_cli_aliases=set(['testalias','permissions']),
  multivalue=True, required=False, values=(u'read', u'search',
  u'compare', u'write', u'add', u'delete', u'all'))
  ipalib/plugins/permission.py:
   StrEnum(
   'ipapermright*',
   cli_name='right',
   deprecated_cli_aliases={'permissions','testalias'},
   label=_('Granted
  rights'),
   doc=_('Rights to grant
  '
 '(read, search, compare, write, add, delete,
  all)'),
   values=(u'read', u'search',
  u'compare',
   u'write', u'add', u'delete',
  u'all'),
   ),
  don't know if it is a problem anyways
- other tests(cli, webui) works fine for me
- unit tests related to this ran as expected
  so besides the multiple deprecated_cli_aliases issue, it's an ACK
 
 It looks like you've edited API.txt by hand and forgot a space after
 the comma in ['testalias','permissions']. Does it work if you use
 makeapi to regenerate API.txt?
 

You are right, my mistake, with ./makeapi, it works, even when the CLI
got this for parameters: 
--right=read --permission=search --testparam=write

ACK

Greets
Adam

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

Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right

2014-03-21 Thread Petr Viktorin 

On 03/21/2014 12:10 PM, Misnyovszki Adam wrote:

On Fri, 21 Mar 2014 11:14:43 +0100
Petr Viktorin pvikt...@redhat.com wrote:


On 03/20/2014 07:20 PM, Misnyovszki Adam wrote:

On Tue, 18 Mar 2014 12:02:06 +0100
Petr Viktorin pvikt...@redhat.com wrote:


Hello,
This renames --permissions to --right. The old name is kept as a
deprecated alias.
FreeIPA didn't have a mechanism for doing this, so I added one.
Also, while I was digging around in this part, I made the new
IntEnum (and all future Enums) act like StrEnum in --help output.


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



499 ACK
500 ACK
501 ACK
   - although should it allow mixing deprecated and current
aliases(eg --permission=read --right=write)?


You're right, this is a strange edge case, but detecting this would
need need a much more complicated approach than sharing the option's
`dest`. I don't think it's worth it.


   - works fine with cli / webui also
   - help displays nicely
502
   - tested with more than one deprecated alias - API.txt validation
 doesn't match, although it has the same output:
Got StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias', 'permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))
Expected StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))

API.txt:
option: StrEnum('ipapermright', attribute=True,
cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))
ipalib/plugins/permission.py:
  StrEnum(
  'ipapermright*',
  cli_name='right',
  deprecated_cli_aliases={'permissions','testalias'},
  label=_('Granted
rights'),
  doc=_('Rights to grant
'
'(read, search, compare, write, add, delete,
all)'),
  values=(u'read', u'search',
u'compare',
  u'write', u'add', u'delete',
u'all'),
  ),
don't know if it is a problem anyways
   - other tests(cli, webui) works fine for me
   - unit tests related to this ran as expected
so besides the multiple deprecated_cli_aliases issue, it's an ACK


It looks like you've edited API.txt by hand and forgot a space after
the comma in ['testalias','permissions']. Does it work if you use
makeapi to regenerate API.txt?



You are right, my mistake, with ./makeapi, it works, even when the CLI
got this for parameters:
--right=read --permission=search --testparam=write

ACK


Thanks for the review!


Greets
Adam



Adam commented in person that I should bump VERSION; I did that as an 
additional one-liner change.


Pushed to master: 801b2fd4588b8b40b74140f26b12eb190306d260


--
Petr³
From fb0ace5ad34cf702f240de64664da2935d4accd2 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 18 Mar 2014 10:15:55 +0100
Subject: [PATCH] permission CLI: rename --permissions to --right

The old name is kept as a deprecated alias.

https://fedorahosted.org/freeipa/ticket/4231
---
 API.txt  | 6 +++---
 VERSION  | 4 ++--
 ipalib/plugins/permission.py | 5 +++--
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/API.txt b/API.txt
index 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..ca9a73e510e6426309ac6c47dab3be763a0d0490 100644
--- a/API.txt
+++ b/API.txt
@@ -2333,7 +2333,7 @@ command: permission_add
 option: Str('filter', attribute=False, cli_name='filter', multivalue=True, required=False)
 option: StrEnum('ipapermbindruletype', attribute=True, autofill=True, cli_name='bindtype', default=u'permission', multivalue=False, required=True, values=(u'permission', u'all', u'anonymous'))
 option: DNOrURL('ipapermlocation', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
-option: StrEnum('ipapermright', attribute=True, cli_name='permissions', multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
+option: StrEnum('ipapermright', attribute=True, cli_name='right', deprecated_cli_aliases=set(['permissions']), multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
 option: DNParam('ipapermtarget', attribute=True, cli_name='target', multivalue=False, required=False)
 option: Str('ipapermtargetfilter', attribute=True, cli_name='rawfilter', multivalue=True, required=False)
 option: Str('memberof', alwaysask=True, attribute=False, autofill=False, cli_name='memberof', multivalue=True, query=False, required=False)
@@ -2392,7 +2392,7 @@ command: permission_find
 option: Str('ipapermexcludedattr', attribute=True, autofill=False,

Re: [Freeipa-devel] [PATCHES] 0499-0502 permission CLI: rename --permissions to --right

2014-03-20 Thread Misnyovszki Adam 
On Tue, 18 Mar 2014 12:02:06 +0100
Petr Viktorin pvikt...@redhat.com wrote:

 Hello,
 This renames --permissions to --right. The old name is kept as a 
 deprecated alias.
 FreeIPA didn't have a mechanism for doing this, so I added one.
 Also, while I was digging around in this part, I made the new IntEnum 
 (and all future Enums) act like StrEnum in --help output.
 
 
 https://fedorahosted.org/freeipa/ticket/4231
 

499 ACK
500 ACK
501 ACK
 - although should it allow mixing deprecated and current aliases(eg
   --permission=read --right=write)?
 - works fine with cli / webui also
 - help displays nicely
502
 - tested with more than one deprecated alias - API.txt validation
   doesn't match, although it has the same output:
Got StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias', 'permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all')) 
Expected StrEnum('ipapermright', attribute=True, cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))

API.txt: 
option: StrEnum('ipapermright', attribute=True,
cli_name='right',
deprecated_cli_aliases=set(['testalias','permissions']),
multivalue=True, required=False, values=(u'read', u'search',
u'compare', u'write', u'add', u'delete', u'all'))
ipalib/plugins/permission.py:
StrEnum(
'ipapermright*',
cli_name='right',   
deprecated_cli_aliases={'permissions','testalias'}, 
label=_('Granted
rights'),  
doc=_('Rights to grant
'
  '(read, search, compare, write, add, delete,
all)'),  
values=(u'read', u'search',
u'compare', 
u'write', u'add', u'delete',
u'all'),   
), 
don't know if it is a problem anyways
 - other tests(cli, webui) works fine for me
 - unit tests related to this ran as expected
so besides the multiple deprecated_cli_aliases issue, it's an ACK

Greets
Adam

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