On 05/30/2013 02:17 PM, Ana Krivokapic wrote:
On 05/13/2013 05:42 PM, Tomas Babej wrote:
On 05/10/2013 04:39 PM, Tomas Babej wrote:
Hi,
this patcheset deals with https://fedorahosted.org/freeipa/ticket/3602
See commit messages for details.
Tomas
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
I noticed during further development that logic in
interactive_prompt_callback did not follow the pre_callback logic
precisely.
Fixed patches attached.
Tomas
Hi,
Patches work fine.
I would suggest incorporating a fix for ticket
https://fedorahosted.org/freeipa/ticket/3634 into this patchset. The
issue from ticket #3634 is closely connected to this one, and with the
introduction of prompt_param() functionality, including a fix for it
would require minimal effort. You can look at my patch
(https://www.redhat.com/archives/freeipa-devel/2013-May/msg00297.html)
and if you think the approach is right, adjust accordingly and
incorporate it in your patchset.
Other (minor) comments:
* The last change in ipalib/plugins/idrange.py seems like you wanted
to fix the fact that the lines weren't properly indented (they weren't
multiples of 4). However, you also need to fix the previous line
(raise ...).
* There are a lot of unused imports in ipalib/frontend.py. Since you
are already touching imports in your patch, could you clean up the
unused imports as well.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
I addressed the minor issues. Updated patches are attached.
Regarding your patch, I agree. I sent a reply to its thread.
Tomas
From cfe01bb1984ac838237c1a3d63e492b8e0628d94 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 9 May 2013 14:47:29 +0200
Subject: [PATCH 55/55] Incorporate interactive prompts in idrange-add
In idrange-add command, ensure that RID base is prompted for
in the interactive mode if domain SID or domain name was
specified.
If domain name nor SID was specified, make sure rid base is
prompted for if secondary rid base was specified and vice versa.
https://fedorahosted.org/freeipa/ticket/3602
---
ipalib/plugins/idrange.py | 41 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index d548794428fbbc7981112d6c441c980fd9e06157..2a5415d077151d8b76ba869460915a5fcae8b89a 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -361,6 +361,41 @@ class idrange_add(LDAPCreate):
msg_summary = _('Added ID range "%(value)s"')
+ def interactive_prompt_callback(self, kw):
+ """
+ Ensure that rid-base is prompted for when dom-sid is specified.
+
+ Also ensure that secondary-rid-base is prompted for when rid-base is
+ specified and vice versa, in case that dom-sid was not specified.
+ """
+
+ # dom-sid can be specified using dom-sid or dom-name options
+
+ # it can be also set using --setattr or --addattr, in these cases
+ # we will not prompt, but raise an ValidationError later
+
+ dom_sid_set = any(dom_id in kw for dom_id in
+ ('ipanttrusteddomainname', 'ipanttrusteddomainsid'))
+
+ rid_base_set = 'ipabaserid' in kw
+ secondary_rid_base_set = 'ipasecondarybaserid' in kw
+
+ # Prompt for RID base if domain SID / name was given
+ if dom_sid_set and not rid_base_set:
+ value = self.prompt_param(self.params['ipabaserid'])
+ kw.update(dict(ipabaserid=value))
+
+ if not dom_sid_set:
+ # Prompt for secondary RID base if RID base was given
+ if rid_base_set and not secondary_rid_base_set:
+ value = self.prompt_param(self.params['ipasecondarybaserid'])
+ kw.update(dict(ipasecondarybaserid=value))
+
+ # Symetrically, prompt for RID base if secondary RID base was given
+ if not rid_base_set and secondary_rid_base_set:
+ value = self.prompt_param(self.params['ipabaserid'])
+ kw.update(dict(ipabaserid=value))
+
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
assert isinstance(dn, DN)
@@ -414,9 +449,9 @@ class idrange_add(LDAPCreate):
entry_attrs['ipabaserid'],
entry_attrs['ipasecondarybaserid'],
entry_attrs['ipaidrangesize']):
- raise errors.ValidationError(name='ID Range setup',
- error=_("Primary RID range and secondary RID range"
- " cannot overlap"))
+ raise errors.ValidationError(name='ID Range setup',
+ error=_("Primary RID range and secondary RID range"
+ " cannot overlap"))
entry_attrs['objectclass'].append('ipadomainidrange')
--
1.8.1.4
From 40c6eb99b42d6ddd2ba518a4a787cbdf609467bd Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 9 May 2013 15:36:41 +0200
Subject: [PATCH 54/55] Add prompt_param method to avoid code duplication
Extracted common code from ipalib/plugins/cli.py and
ipalib/plugins/dns.py that provided way to prompt user
for the value of specific attribute.
Added prompt_param method to Command class in ipalib/frontend.py
Done as part of https://fedorahosted.org/freeipa/ticket/3602
---
ipalib/cli.py | 25 ++++++++++++-------------
ipalib/frontend.py | 37 +++++++++++++++++++++++++++++++------
ipalib/plugins/dns.py | 33 +++++++++++----------------------
3 files changed, 54 insertions(+), 41 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index c4b4492a59ff0a1a1b69ad045b253fdd30833eb9..5f02e929fe0df7051f4bb925a960678d780d4883 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1178,11 +1178,13 @@ class cli(backend.Executioner):
``self.env.prompt_all`` is ``True``, this method will prompt for any
params that have a missing values, even if the param is optional.
"""
+
honor_alwaysask = True
for param in cmd.params():
if param.alwaysask and param.name in kw:
honor_alwaysask = False
break
+
for param in cmd.params():
if (param.required and param.name not in kw) or \
(param.alwaysask and honor_alwaysask) or self.env.prompt_all:
@@ -1196,19 +1198,16 @@ class cli(backend.Executioner):
)
else:
default = cmd.get_default_of(param.name, **kw)
- error = None
- while True:
- if error is not None:
- self.Backend.textui.print_prompt_attribute_error(unicode(param.label),
- unicode(error))
- raw = self.Backend.textui.prompt(param.label, default, optional=param.alwaysask or not param.required)
- try:
- value = param(raw, **kw)
- if value is not None:
- kw[param.name] = value
- break
- except (ValidationError, ConversionError), e:
- error = e.error
+ optional = param.alwaysask or not param.required
+
+ value = cmd.prompt_param(param,
+ default=default,
+ optional=optional,
+ kw=kw)
+
+ if value is not None:
+ kw[param.name] = value
+
elif param.password and kw.get(param.name, False) is True:
kw[param.name] = self.Backend.textui.prompt_password(
param.label, param.confirm
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 0331dc52b49b04dd7cf2205ea24e50ba8d944fbd..6b35a0850bd9cd700ffb2b390cf56111861c96f3 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -22,19 +22,18 @@ Base classes for all front-end plugins.
"""
import re
-import inspect
from distutils import version
from ipapython.version import API_VERSION
from ipapython.ipa_log_manager import root_logger
-from base import lock, check_name, NameSpace
+from base import NameSpace
from plugable import Plugin, is_production_mode
-from parameters import create_param, parse_param_spec, Param, Str, Flag, Password
+from parameters import create_param, Param, Str, Flag
from output import Output, Entry, ListOfEntries
-from text import _, ngettext
+from text import _
from errors import (ZeroArgumentError, MaxArgumentError, OverlapError,
- RequiresRoot, VersionError, RequirementError, OptionError, InvocationError)
-from constants import TYPE_ERROR
+ VersionError, OptionError, InvocationError,
+ ValidationError, ConversionError)
from ipalib import messages
@@ -560,6 +559,32 @@ class Command(HasParam):
if name in params:
yield(name, params[name])
+ def prompt_param(self, param, default=None, optional=False, kw=dict(),
+ label=None):
+ """
+ Prompts the user for the value of given parameter.
+
+ Returns the parameter instance.
+ """
+
+ if label is None:
+ label = param.label
+
+ while True:
+ raw = self.Backend.textui.prompt(label, default, optional=optional)
+
+ # Backend.textui.prompt does not fill in the default value,
+ # we have to do it ourselves
+ if not raw.strip():
+ raw = default
+
+ try:
+ return param(raw, **kw)
+ except (ValidationError, ConversionError), e:
+ # Display error and prompt again
+ self.Backend.textui.print_prompt_attribute_error(unicode(label),
+ unicode(e.error))
+
def normalize(self, **kw):
"""
Return a dictionary of normalized values.
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index fbc445215dac583cac91de6cb9b33ee82e1aeffd..621d60ec9a5304c12e545a6a495179f6069712c9 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -759,26 +759,16 @@ class DNSRecord(Str):
return tuple(self._convert_dnsrecord_extra(extra) for extra in self.extra)
- def __get_part_param(self, backend, part, output_kw, default=None):
+ def __get_part_param(self, cmd, part, output_kw, default=None):
name = self.part_name_format % (self.rrtype.lower(), part.name)
label = self.part_label_format % (self.rrtype, unicode(part.label))
optional = not part.required
- while True:
- try:
- raw = backend.textui.prompt(label,
- optional=optional,
- default=default)
- if not raw.strip():
- raw = default
+ output_kw[name] = cmd.prompt_param(part,
+ optional=optional,
+ label=label)
- output_kw[name] = part(raw)
- break
- except (errors.ValidationError, errors.ConversionError), e:
- backend.textui.print_prompt_attribute_error(
- unicode(label), unicode(e.error))
-
- def prompt_parts(self, backend, mod_dnsvalue=None):
+ def prompt_parts(self, cmd, mod_dnsvalue=None):
mod_parts = None
if mod_dnsvalue is not None:
mod_parts = self._get_part_values(mod_dnsvalue)
@@ -793,18 +783,17 @@ class DNSRecord(Str):
else:
default = None
- self.__get_part_param(backend, part, user_options, default)
+ self.__get_part_param(cmd, part, user_options, default)
return user_options
- def prompt_missing_parts(self, backend, kw, prompt_optional=False):
+ def prompt_missing_parts(self, cmd, kw, prompt_optional=False):
user_options = {}
if self.parts is None:
return user_options
for part in self.parts:
name = self.part_name_format % (self.rrtype.lower(), part.name)
- label = self.part_label_format % (self.rrtype, unicode(part.label))
if name in kw:
continue
@@ -814,7 +803,7 @@ class DNSRecord(Str):
continue
default = part.get_default(**kw)
- self.__get_part_param(backend, part, user_options, default)
+ self.__get_part_param(cmd, part, user_options, default)
return user_options
@@ -2395,7 +2384,7 @@ class dnsrecord_add(LDAPCreate):
# it can be used to fill all required params by itself
new_kw = {}
for rrparam in self.obj.iterate_rrparams_by_parts(kw, skip_extra=True):
- user_options = rrparam.prompt_missing_parts(self.Backend, kw,
+ user_options = rrparam.prompt_missing_parts(self, kw,
prompt_optional=False)
new_kw.update(user_options)
kw.update(new_kw)
@@ -2437,7 +2426,7 @@ class dnsrecord_add(LDAPCreate):
continue
ok = True
- user_options = param.prompt_parts(self.Backend)
+ user_options = param.prompt_parts(self)
kw.update(user_options)
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -2698,7 +2687,7 @@ class dnsrecord_mod(LDAPUpdate):
mod_value = self.Backend.textui.prompt_yesno(
_("Modify %(name)s '%(value)s'?") % dict(name=param.label, value=rec_value), default=False)
if mod_value is True:
- user_options = param.prompt_parts(self.Backend, mod_dnsvalue=rec_value)
+ user_options = param.prompt_parts(self, mod_dnsvalue=rec_value)
kw[param.name] = [rec_value]
kw.update(user_options)
--
1.8.1.4
From 18d4a538d052e4867031f1522a87733b1be720b1 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 9 May 2013 14:50:52 +0200
Subject: [PATCH 53/55] Remove redundant check for env.interactive
Fixed as part of
https://fedorahosted.org/freeipa/ticket/3602
---
ipalib/cli.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 84dea2e52f4729140602d6968a26bca1b47e0977..c4b4492a59ff0a1a1b69ad045b253fdd30833eb9 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1043,7 +1043,6 @@ class cli(backend.Executioner):
"""Get the keyword arguments for a Command"""
if self.env.interactive:
self.prompt_interactively(cmd, kw)
- if self.env.interactive:
try:
callbacks = cmd.get_callbacks('interactive_prompt')
except AttributeError:
--
1.8.1.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
