<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 07/14/2016 11:25 AM, Lenka Doudova
wrote:<br>
</div>
<blockquote
cite="mid:8d55e1b4-794e-db99-eb79-191ff1aee668@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 07/14/2016 09:20 AM, Lenka Doudova
wrote:<br>
</div>
<blockquote
cite="mid:fdd5a89f-6774-08ba-e810-a621cfa78669@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 07/13/2016 04:48 PM, Milan Kubík
wrote:<br>
</div>
<blockquote
cite="mid:a17fed62-31ec-2998-5b54-ebbd442dd325@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 07/11/2016 01:34 PM, Lenka
Doudova wrote:<br>
</div>
<blockquote
cite="mid:fa2afe96-1a10-7727-2282-028396d42ed7@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 07/08/2016 02:24 PM, Milan
Kubík wrote:<br>
</div>
<blockquote
cite="mid:8eb18fba-f6dd-d4e9-d2c7-dfd85c035903@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 07/01/2016 05:13 PM, Lenka
Doudova wrote:<br>
</div>
<blockquote
cite="mid:0aa06017-fcd2-5867-73a9-4dfde6223215@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 07/01/2016 02:42 PM,
Milan Kubík wrote:<br>
</div>
<blockquote
cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com"
type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 06/16/2016 03:23 PM,
Lenka Doudova wrote:<br>
</div>
<blockquote
cite="mid:766fd34a-b83b-1105-57d0-bd51420720a7@redhat.com"
type="cite">Hi, <br>
<br>
attached are tests for authentication indicators.
Please note: <br>
<br>
1. newly created service tracker is not exactly
complete, list of unimplemented methods is in doc.
These methods can be filled in when existing
declarative tests are refactored. <br>
<br>
2. patch 0015 depends on 0014, so it should not be
pushed without it. <br>
<br>
<br>
Lenka <br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
<br>
<p>patch 0014:</p>
<p>In the update method, what happens when the updated
attributes contain addattr? It is not clear to me.
Is it necessary?</p>
</blockquote>
Example: <br>
ipa service-mod SRV --addattr="authind=radius"<br>
<br>
Result:<br>
The way the tracker works, this adds <i>u'addattr="authind=radius"'</i>
to the list of expected results (result of <i>self.attrs.update(updates)</i>.
Of course nothing like that appears anywhere, so in case
there's the <i>--addattr</i> option, it's necessary to
ensure it won't get to the <i>self.attrs</i> atribute.<br>
<br>
<blockquote
cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com"
type="cite">
<p>patch 0015:</p>
<p>host1 and service2 do not tell anything about the
purpose of the fixture. Please assign more
descriptive names to them.<br>
Why do the fixtures have 'function' scope? Does the
service entry exist during the second and third test
case?</p>
</blockquote>
Renamed.<br>
<blockquote
cite="mid:556ae9fd-a110-c5a3-b80a-f45d80b01c16@redhat.com"
type="cite">
<p>patch 0016:</p>
<p>Per offline discussion, admin user has no special
privileges here, LGTM.<br>
</p>
<pre class="moz-signature" cols="72">--
Milan Kubik</pre>
</blockquote>
<br>
Thanks for review, fixed patches (14.2 and 15.2)
attached.<br>
Lenka<br>
</blockquote>
<br>
<p>NACK,</p>
<p>the update method of ServiceTracker creates the entry
if it doesn't exist. Why? I know the base class has this
problem also [1], though.<br>
Given this will be addressed, the fixtures in the xmlrpc
test will fail since the fixture scope is wrong -
function instead of class.</p>
<p>[1]: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://fedorahosted.org/freeipa/ticket/6045">https://fedorahosted.org/freeipa/ticket/6045</a><br>
</p>
<pre class="moz-signature" cols="72">--
Milan Kubik</pre>
</blockquote>
Hi,<br>
<br>
fixed patch attached. I chose to leave the scope as it was
(in case one test breaks and entry is not even created, the
other tests can still be successful), but I removed the
creation command from ServiceTracker update method and
called it directly in the tests, so there shouldn't be
hidden actions.<br>
<br>
Lenka<br>
</blockquote>
<br>
<p>Thanks for the updated patches. There are still some issues
I haven't noticed before:</p>
<p>service tracker: track_create method doesn't set
self.exists flag which is needed</p>
<p>In the xmlrpc test method `test_adding_second_indicator`
uses 'addattr' to set the indicator, why?</p>
<pre class="moz-signature" cols="72">--
Milan Kubik</pre>
</blockquote>
<br>
Hi,<br>
fixes for comments in attached patches.<br>
After offline discussion, I removed the usage of "addattr" and
replaced it with test to add two indicators in one command.<br>
<br>
Lenka<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
One more problem was pointed out to me offline, attaching changed
patch 0014.<br>
<br>
Lenka<br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
Resending the complete patch set.<br>
L.<br>
</body>
</html>