[Freeipa-devel] [PATCH 0018] Make service naming in ipa-server-install consistent

[Martin Kosek]

On 10/19/2012 02:49 PM, Tomas Babej wrote:
> On 10/19/2012 01:44 PM, Martin Kosek wrote:
>> On 10/19/2012 01:26 PM, Tomas Babej wrote:
>>> On 10/18/2012 11:27 AM, Martin Kosek wrote:
>>>> On 10/11/2012 05:11 PM, Tomas Babej wrote:
>>>>> On 10/11/2012 12:32 PM, Martin Kosek wrote:
>>>>>> On 10/11/2012 12:26 PM, Tomas Babej wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch forces more consistency into ipa-server-install output. All
>>>>>>> descriptions of services that are not instances of
>>>>>>> SimpleServiceInstance are now in the following format:
>>>>>>>
>>>>>>> <Description> (<Service Name>)
>>>>>>>
>>>>>>> Furthermore, start_creation method has been modified to support
>>>>>>> custom start and end messages.
>>>>>>>
>>>>>>> Sample output produced by this patch attached.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/3059
>>>>>>>
>>>>>>> Tomas
>>>>>>>
>>>>>> Just based on reading the patch, this does not look right:
>>>>>>
>>>>>> -        self.start_creation("Configuring certificate server", 210)
>>>>>> +        self.start_creation("Configuring directory server for the CA",
>>>>>> +            end_message="Done configuring directory server for the CA",
>>>>>> +            show_service_name=True, runtime=210)
>>>>>>
>>>>>> Martin
>>>>>>
>>>>> Thanks, glitch fixed.
>>>>>
>>>>> Tomas
>>>> Ok, I managed to get the patch a proper review. I like the result, in the
>>>> console, but I still not entirely satisfied with the patch, looks
>>>> over-engineered to me + there is a lot of duplication with "Configuring
>>>> %(service)s" and "Done configuring %(service)s" messages.
>>>>
>>>> These messages could be easily generated only based on name of a service
>>>> (self.service_name, we got that) and a new "friendly service name" or
>>>> description.
>>>>
>>>> If we add description this way:
>>>>
>>>> class KrbInstance(service.Service):
>>>>       def __init__(self, fstore=None):
>>>>           service.Service.__init__(self, "krb5kdc", description="Kerberos
>>>> KDC")
>>>> ...
>>>>
>>>> the start_creation could be very simple:
>>>> ...
>>>> self.start_creation(runtime=30)
>>>> ...
>>>>
>>>> All messages could be simply generated by the Service class just based on
>>>> self.service_name and self.description with having the same output as you do
>>>> now.
>>>>
>>>> Martin
>>> I simplified the approach as you suggested. However, I kept optional
>>> start_message and end_message parameters in case we would want
>>> to specify the messages instead of using the pre-generated ones.
>>> This is used in baseupdate.py and upgradeinstance.py
>>>
>>> More info in the documentation of start_creation() function.
>>>
>>> Tomas
>> NACK. Tried running ./make-lint with your patch, got thousands of errors...
> More like one error thousand times :) I somehow botched up rebasing
> the patch, I swear that I corrected that indentation issue beforehand.
> Nevermind, thanks for checking. ./make-lint runs clean now :)
>>
>> But the approach looks OK, I am just thinking that default value for
>> show_service_name of start_creation() should be True as it is the most widely
>> used value - you would not have to specify it everytime you call
>> start_creation() in *instance.py files.
>>
>> Martin
> Default values switched.
> 
> Tomas

NACK. Server installation crashed with the first step:

# ipa-server-install

Configuring NTP daemon (ntpd)
  [1/4]: stopping ntpd
  [2/4]: writing configuration
  [3/4]: configuring ntpd to start on boot
  [4/4]: starting ntpd
Unexpected error - see /var/log/ipaserver-install.log for details:
TypeError: expected a character buffer object


ipaserver-install.log:

2012-10-19T12:59:37Z INFO   File
"/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",  line
614, in run_script
    return_value = main_function()

  File "/sbin/ipa-server-install", line 904, in main
    ntp.create_instance()

  File "/usr/lib/python2.7/site-packages/ipaserver/install/ntpinstance.py",
line 158, in create_instance
    self.start_creation()

  File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line
358, in start_creation
    self.print_msg(end_message)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line
295, in print_msg
    print_msg(message, self.output_fd)

  File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line
60, in print_msg
    output_fd.write(message)

2012-10-19T12:59:37Z INFO The ipa-server-install command failed, exception:
TypeError: expected a       character buffer object


Martin