Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
On Wed, 2011-06-01 at 16:18 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Fri, 2011-05-27 at 16:25 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote: Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? Done. At least user will know that we have a feature like that to offer. And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? Actually, this is a very good idea, I like it. I updated the patch so that the user can only do yes/no decision in ipa dnsrecord-del interactive mode. This makes dnsrecord-del interactive mode very usable. The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). Done. '' shouldn't be returned as I return the value of default if it is not None. But yes, it needed more documenting. Updated patch is attached. It may need some language corrections, I am no native speaker. Martin Not to be too pedantic but... The result variable isn't really used, a while True: would suffice. I'm not really sure what the purpose of default = None is. I think a True/False is more appropriate, this 3rd answer of a binary question is confusing. I fixed the result variable. This was a left-over from function evolution. I am not sure why is the yes/no function still confusing. Maybe I miss something. I improved function help a bit. But let me explain: If default is None, that means that there is no default answer to yes/no question and user has to answer either y or n. He cannot skip the answer and is prompted until the answer is given. When default is True, user can just enter empty answer, which is treated as yes and True is returned. When default is False and user enters empty answer, it is treated as no and False is returned. None shouldn't be returned at all... (Maybe only in a case of an error) Martin Wow, this is very nice indeed. Ack. rob Thanks :-) Pushed to master. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
Martin Kosek wrote: On Fri, 2011-05-27 at 16:25 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote: Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? Done. At least user will know that we have a feature like that to offer. And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? Actually, this is a very good idea, I like it. I updated the patch so that the user can only do yes/no decision in ipa dnsrecord-del interactive mode. This makes dnsrecord-del interactive mode very usable. The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). Done. '' shouldn't be returned as I return the value of default if it is not None. But yes, it needed more documenting. Updated patch is attached. It may need some language corrections, I am no native speaker. Martin Not to be too pedantic but... The result variable isn't really used, a while True: would suffice. I'm not really sure what the purpose of default = None is. I think a True/False is more appropriate, this 3rd answer of a binary question is confusing. I fixed the result variable. This was a left-over from function evolution. I am not sure why is the yes/no function still confusing. Maybe I miss something. I improved function help a bit. But let me explain: If default is None, that means that there is no default answer to yes/no question and user has to answer either y or n. He cannot skip the answer and is prompted until the answer is given. When default is True, user can just enter empty answer, which is treated as yes and True is returned. When default is False and user enters empty answer, it is treated as no and False is returned. None shouldn't be returned at all... (Maybe only in a case of an error) Martin Wow, this is very nice indeed. Ack. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote: Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? Done. At least user will know that we have a feature like that to offer. And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? Actually, this is a very good idea, I like it. I updated the patch so that the user can only do yes/no decision in ipa dnsrecord-del interactive mode. This makes dnsrecord-del interactive mode very usable. The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). Done. '' shouldn't be returned as I return the value of default if it is not None. But yes, it needed more documenting. Updated patch is attached. It may need some language corrections, I am no native speaker. Martin From d12e9389f280ac8848f3d61d3e382d2d220319e3 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 26 May 2011 09:55:53 +0200 Subject: [PATCH] Improve interactive mode for DNS plugin Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 --- ipalib/cli.py | 41 +++ ipalib/plugins/baseldap.py | 42 ipalib/plugins/dns.py | 159 ++-- 3 files changed, 222 insertions(+), 20 deletions(-) diff --git a/ipalib/cli.py b/ipalib/cli.py index 99f236bb4103c8524320b03aa4a311689ecef8e8..dc3a730b460ce0a033a9418bf1cc6535b5d6ddff 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -527,6 +527,44 @@ class textui(backend.Backend): return None return self.decode(data) +def prompt_yesno(self, label, default=None): + +Prompt user for yes/no input. This method returns True/False according +to user response. + +If Default parameter is not None, its value is returned if user pass +no input to yes/no prompt. + +If Default parameter is None, user is asked until a correct answer is +provided (yes/no) or the program is not terminated. + + +default_prompt = None +if default is not None: +if default: +default_prompt = Yes +else: +default_prompt = No + +if default_prompt: +prompt = u'%s Yes/No (default %s): ' % (label, default_prompt) +else: +prompt = u'%s Yes/No: ' % label + +result=None +while result is None: +try: +data = raw_input(self.encode(prompt)).lower() +except EOFError: +return None + +if data in (u'yes', u'y'): +return True +elif data in ( u'n', u'no'): +return False +elif default is not None and data == u'': +return default + def prompt_password(self, label): Prompt user for a password or read it in via stdin depending @@ -1032,6 +1070,9 @@ class cli(backend.Executioner): param.label ) +for callback in getattr(cmd, 'INTERACTIVE_PROMPT_CALLBACKS', []): +callback(kw) + def load_files(self, cmd, kw): Load files from File parameters. diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 43533c8c969e368f450cb049235816db8a954b46..1823e08b03bc942cef679ed6d5dbb0d1f7ce05e6 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -450,12 +450,17 @@ class CallbackInterface(Method): self.__class__.POST_CALLBACKS = [] if not hasattr(self.__class__, 'EXC_CALLBACKS'): self.__class__.EXC_CALLBACKS = [] +if not
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
On Thu, 2011-05-26 at 21:00 +0200, Jan Cholasta wrote: On 26.5.2011 14:32, Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 ACK, works fine. Just a minor thing: $ git apply freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:33: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:41: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:289: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:193: new blank line at EOF. + warning: 4 lines add whitespace errors. Honza Yeah, I fixed these. Nothing in my workflow reports me such errors, I must enhance my .vimrc to do it for me. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
Martin Kosek wrote: On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote: Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? Done. At least user will know that we have a feature like that to offer. And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? Actually, this is a very good idea, I like it. I updated the patch so that the user can only do yes/no decision in ipa dnsrecord-del interactive mode. This makes dnsrecord-del interactive mode very usable. The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). Done. '' shouldn't be returned as I return the value of default if it is not None. But yes, it needed more documenting. Updated patch is attached. It may need some language corrections, I am no native speaker. Martin Not to be too pedantic but... The result variable isn't really used, a while True: would suffice. I'm not really sure what the purpose of default = None is. I think a True/False is more appropriate, this 3rd answer of a binary question is confusing. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
On Fri, 2011-05-27 at 16:25 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote: Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? Done. At least user will know that we have a feature like that to offer. And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? Actually, this is a very good idea, I like it. I updated the patch so that the user can only do yes/no decision in ipa dnsrecord-del interactive mode. This makes dnsrecord-del interactive mode very usable. The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). Done. '' shouldn't be returned as I return the value of default if it is not None. But yes, it needed more documenting. Updated patch is attached. It may need some language corrections, I am no native speaker. Martin Not to be too pedantic but... The result variable isn't really used, a while True: would suffice. I'm not really sure what the purpose of default = None is. I think a True/False is more appropriate, this 3rd answer of a binary question is confusing. I fixed the result variable. This was a left-over from function evolution. I am not sure why is the yes/no function still confusing. Maybe I miss something. I improved function help a bit. But let me explain: If default is None, that means that there is no default answer to yes/no question and user has to answer either y or n. He cannot skip the answer and is prompted until the answer is given. When default is True, user can just enter empty answer, which is treated as yes and True is returned. When default is False and user enters empty answer, it is treated as no and False is returned. None shouldn't be returned at all... (Maybe only in a case of an error) Martin From 13621b72cc9d45128e1438d7decf472f14eeb3e1 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 26 May 2011 09:55:53 +0200 Subject: [PATCH] Improve interactive mode for DNS plugin Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 --- ipalib/cli.py | 44 ipalib/plugins/baseldap.py | 42 ipalib/plugins/dns.py | 159 ++-- 3 files changed, 225 insertions(+), 20 deletions(-) diff --git a/ipalib/cli.py b/ipalib/cli.py index 99f236bb4103c8524320b03aa4a311689ecef8e8..5e1365dc37c290eeeb38aa6266a9798854a132ee 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -527,6 +527,47 @@ class textui(backend.Backend): return None return self.decode(data) +def prompt_yesno(self, label, default=None): + +Prompt user for yes/no input. This method returns True/False according +to user response. + +Parameter default should be True, False or None + +If Default parameter is not None, user can enter an empty input instead +of Yes/No answer. Value passed to Default is returned in that case. + +If Default parameter is None, user is asked for Yes/No answer until +a correct answer is provided. Answer is then returned. + +In case of an error, a None value may returned + + +default_prompt = None +if default is not None: +if default: +default_prompt = Yes +else: +default_prompt = No + +if default_prompt: +prompt = u'%s Yes/No (default %s): ' % (label, default_prompt) +else: +prompt = u'%s Yes/No: ' % label + +while True: +try: +data =
[Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 From 25ece379e5982f28797e2fe5928511240bb0bfa5 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 26 May 2011 09:55:53 +0200 Subject: [PATCH] Improve interactive mode for DNS plugin Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 --- ipalib/cli.py | 35 ipalib/plugins/baseldap.py | 43 +++ ipalib/plugins/dns.py | 129 +--- 3 files changed, 188 insertions(+), 19 deletions(-) diff --git a/ipalib/cli.py b/ipalib/cli.py index 99f236bb4103c8524320b03aa4a311689ecef8e8..b33449842c3d3929c2d669b4610ca146a57c8f16 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -527,6 +527,38 @@ class textui(backend.Backend): return None return self.decode(data) +def prompt_yesno(self, label, default=None): + +Prompt user for yes/no input. + + +default_prompt = None +if default is not None: +if default: +default_prompt = Yes +else: +default_prompt = No + + +if default_prompt: +prompt = u'%s Yes/No (default %s): ' % (label, default_prompt) +else: +prompt = u'%s Yes/No: ' % label + +result=None +while result is None: +try: +data = raw_input(self.encode(prompt)).lower() +except EOFError: +return None + +if data in (u'yes', u'y'): +return True +elif data in ( u'n', u'no'): +return False +elif default is not None and data == u'': +return default + def prompt_password(self, label): Prompt user for a password or read it in via stdin depending @@ -1032,6 +1064,9 @@ class cli(backend.Executioner): param.label ) +for callback in getattr(cmd, 'INTERACTIVE_PROMPT_CALLBACKS', []): +callback(kw) + def load_files(self, cmd, kw): Load files from File parameters. diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 43533c8c969e368f450cb049235816db8a954b46..0cc11a8fdc9f312288928b96d53ba10d662adc32 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -450,12 +450,17 @@ class CallbackInterface(Method): self.__class__.POST_CALLBACKS = [] if not hasattr(self.__class__, 'EXC_CALLBACKS'): self.__class__.EXC_CALLBACKS = [] +if not hasattr(self.__class__, 'INTERACTIVE_PROMPT_CALLBACKS'): +self.__class__.INTERACTIVE_PROMPT_CALLBACKS = [] if hasattr(self, 'pre_callback'): self.register_pre_callback(self.pre_callback, True) if hasattr(self, 'post_callback'): self.register_post_callback(self.post_callback, True) if hasattr(self, 'exc_callback'): self.register_exc_callback(self.exc_callback, True) +if hasattr(self, 'interactive_prompt_callback'): +self.register_interactive_prompt_callback( +self.interactive_prompt_callback, True) #pylint: disable=E1101 super(Method, self).__init__() @classmethod @@ -488,6 +493,16 @@ class CallbackInterface(Method): else: klass.EXC_CALLBACKS.append(callback) +@classmethod +def register_interactive_prompt_callback(klass, callback, first=False): +assert callable(callback) +if not hasattr(klass, 'INTERACTIVE_PROMPT_CALLBACKS'): +klass.INTERACTIVE_PROMPT_CALLBACKS = [] +if first: +klass.INTERACTIVE_PROMPT_CALLBACKS.insert(0, callback) +else: +klass.INTERACTIVE_PROMPT_CALLBACKS.append(callback) + def _call_exc_callbacks(self, args, options, exc, call_func, *call_args, **call_kwargs): rv = None for i in xrange(len(getattr(self, 'EXC_CALLBACKS', []))): @@ -635,6 +650,9 @@ class LDAPCreate(CallbackInterface, crud.Create): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc +def interactive_prompt_callback(self, kw): +return + # list of attributes we want exported to JSON
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
On 26.5.2011 14:32, Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 ACK, works fine. Just a minor thing: $ git apply freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:33: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:41: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:289: trailing whitespace. freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:193: new blank line at EOF. + warning: 4 lines add whitespace errors. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin
Martin Kosek wrote: Interactive mode for commands manipulating with DNS records (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances the server framework with new callback for interactive mode, which can be used by commands to inject their own interactive handling. The callback is then used to improve aforementioned commands' interactive mode. https://fedorahosted.org/freeipa/ticket/1018 This works pretty nicely but it seems like with just a bit more it can be great. Can you add some doc examples for how this works? And you display the records now and then prompt for each to delete. Can you combine the two? For example: ipa dnsrecord-del greyoak.com lion No option to delete specific record provided. Delete all? Yes/No (default No): Current DNS record contents: A record: 192.168.166.32 Enter value(s) to remove: [A record]: If we know there is an record why not just prompt for each value yes/no to delete? The yes/no function needs more documentation on what default does too. It appears that the possible values are None/True/False and that None means that '' can be returned (which could still be evaluated as False if this isn't used right). rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
