Re: [Freeipa-devel] [PATCH] 045 Add DNS record modification command

2011-03-31 Thread Martin Kosek
On Wed, 2011-03-30 at 16:52 -0400, Adam Young wrote:
 On 03/30/2011 11:13 AM, Martin Kosek wrote: 
  Since this is a new-feature type patch it should be pushed only to master.
  ---
  The DNS record plugin does not support modification of a record. One
  can only add A type addresses to a DNS record or remove the current
  ones. To actually change a DNS record value it has to be removed and
  then added with a desired value.
  
  This patch adds a new DNS plugin command dnsrecord-mod which enables
  user to:
   - modify a DNS record value (note than DNS record can hold multiple values
 and those will be overwritten)
   - remove a DNS record when an empty value is passed
  
  New tests for this new command have been added to the CLI test suite.
  
  https://fedorahosted.org/freeipa/ticket/1137
  
  
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
 
 
 NACK,
 
 The problem is that if there are 10 A records, and I only want to
 modify one, I have no way to specify which one.
 
 The API should be something like:
 
 ipa dnsrecord-mod ayoung.boston.devel.redhat.com testa  10.10.2.3
 --a-rec=,10.11.12.13
 
 
 Alternatively, we can decide that we are not going to do mod, and have
 the WebUI do a delete and an add:

Hm, that may be a valid use-case. We should discuss how we want the DNS
record modification to behave.

The proposed API is not what we want, since we can modify multiple
attributes at once, e.g.:

ipa dnsrecord-mod DNSZONE DNSRECORD --a-rec=10.0.0.1 ---rec=::1

I can introduce new option --old-DNS_TYPE-rec for each DNS record type
available, e.g. --old-a-rec, --old--rec, --old-srv-rec etc. You
would be able to do:

ipa dnsrecord-mod DNSZONE DNSRECORD --old-a-rec=10.10.2.3
--a-rec=10.11.12.13

This would of course increase the size of this patch. I tried to find
how we treat other multi-value LDAP attributes. In most cases the
behavior is the same like in my first patch (user mail, mobile...) or
the modification is not supported at all (list of privilege
permissions).

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


Re: [Freeipa-devel] [PATCH] admiyo-0217-define-entities-using-builder-and-more-declarative

2011-03-31 Thread Martin Kosek
On Wed, 2011-03-30 at 12:40 -0400, Adam Young wrote:
 On 03/28/2011 05:17 PM, Adam Young wrote: 
  On 03/28/2011 04:56 PM, Adam Young wrote: 
   To give a little more context:  we are llong to split out the
   logic used to define the views of the entities from the reusable
   portion of the toolkit.  This patch introduces a builder object
   which contains the temporary state of the entity build process. 
   
   In the course of writing it, I realized a few things: 
   
   1.  HBAC and SUDO have two small entities and a single large one.
   Thus, it makes sense to group them both into a single file per
   entity.  Both hbac.js and sudo.js should shrink more in the future
   as the custom code gets better refactored and split into reusable
   components and configuration data. 
   
   
   2.  policy.js was a catch all file.  Automount  will grow
   significantly this release, and so should have its own file.   DNS
   is complicated enough that it deserves its own top level js file.
   policy is now reduced to two small entities, both that are very
   clearly policy. 
   
   ___
   Freeipa-devel mailing list
   Freeipa-devel@redhat.com
   https://www.redhat.com/mailman/listinfo/freeipa-devel
  
  
  Self NACK:  jsl and unit test errors need to be fixed first.   Still
  worth reviewing as is, as fixing that will not change the behavior
  or structure of the end patch.
  
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
 
 
 Updated with fix for unit tests.   Note that this requires the NACKed
 version of
 freeipa-admiyo-0216-update-metadata-with-label-changes.patch.  The
 fixes in the unit tests here resolve the test breakage due to the
 metadata updates.

Unfortunately the patch failed to apply to the master, there are some
conflicts in policy.js. Can you please send a rebased patch?

Martin

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


Re: [Freeipa-devel] [PATCH] 759 cache get_ipa_config() output in request context

2011-03-31 Thread Martin Kosek
On Wed, 2011-03-30 at 10:23 -0400, Rob Crittenden wrote:
 Some requests generate multiple calls to get_ipa_config(). This patch 
 caches the return value for this in the request context.
 
 ticket 1023
 
 rob

ACK.

Tested with user mail  config attribute ipadefaultemaildomain.

Martin

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


Re: [Freeipa-devel] [PATCH] 760 don't crash when calculating indirect

2011-03-31 Thread Martin Kosek
On Wed, 2011-03-30 at 10:46 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
  This prevents an internal error when calculating direct vs indirect
  membership.
 
  ticket 1133
 
 
 I accidentally included a change from another patch. Updated patch attached.
 
 rob

I think it is OK. But I would suggest adding some comment to the code -
a reason why we pass the ValueError exception. It may not be
self-explanatory when we return to this code in the future.

Martin

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


Re: [Freeipa-devel] [PATCH] 761 Sort entries on *-find commands

2011-03-31 Thread Martin Kosek
On Wed, 2011-03-30 at 17:14 -0400, Rob Crittenden wrote:
 Sort output on find commands based on the baseldap LDAPSearch class.
 
 A couple tests had to be modified to match the new order.
 
 ticket 794
 
 rob

The patch works fine except the case when entries are being added in
post_callback. Check this search:

ipa permission-find --permissions=write

The result is not sorted. I suggest moving the sort process after the
self.POST_CALLBACKS calls.

What about performance issues? May somebody want to disable the sorting?
(e.g. --nosort option).

Martin

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


Re: [Freeipa-devel] [PATCH] admiyo-0217-define-entities-using-builder-and-more-declarative

2011-03-31 Thread Endi Sukma Dewata

On 3/31/2011 8:43 AM, Adam Young wrote:

rebased both patches


I don't see any code change in the rebased patches, only new commit 
ID's, I hope this is correct.


Some comments (some of which had been discussed over IRC):

1. I ran my Selenium test cases against 215, 216, and 217 together,
   so far there's no failure.

2. There's a IPA.metadata assignment and the like in unit tests, this
   is redundant.

3. In IPA.entity_builder.section(), the current_section should be added
   to the current_facet before adding the fields to do incremental
   construction.

4. In IPA.entity_builder the entity_name can be replaced with
   entity.name to reduce the number of variables.

5. In IPA.entity_builder the standard_associations() can be replaced
   standard_association_facets() for consistency.

6. In the permission entity definition, the 'add_fields' is used
   inconsistently to add a section (i.e. IPA.target_section). The
   solution is either adding 'add_sections' or converting
   IPA.target_sections into widgets. I think adding 'add_sections' is
   simpler because widgets is designed to represent a single attribute.

7. The IPA.entity_builder.details_facet() takes an array of sections
   instead of a spec object. This limits the expandability of the
   builder interface. It should take a spec object with a 'sections'
   attribute containing the array of sections, this would be consistent
   with the other interfaces.

8. In IPA.entity_builder.search_facet(), there's no need to call
   current_facet.init() because all facets will be initialized by
   the entity when IPA.start_entities() is invoked.

9. The IPA.entity_builder could be a singleton because it doesn't take
   any parameters and there's no multi-threading issue.

10. In IPA.details_refresh() it calls IPA.refresh_devel_hook()
to execute a code specifically used by testing. I think ideally
the develop.js should modify the entity_builder instance to generate
details facet with test-specific code. This requires item #9. But
for now at least we should rename IPA.refresh_devel_hook() into
IPA.details_refresh_devel_hook() for clarity.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 759 cache get_ipa_config() output in request context

2011-03-31 Thread Rob Crittenden

Martin Kosek wrote:

On Wed, 2011-03-30 at 10:23 -0400, Rob Crittenden wrote:

Some requests generate multiple calls to get_ipa_config(). This patch
caches the return value for this in the request context.

ticket 1023

rob


ACK.

Tested with user mail  config attribute ipadefaultemaildomain.

Martin


pushed to master and ipa-2-0

rob

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


Re: [Freeipa-devel] [PATCH] admiyo-0217-define-entities-using-builder-and-more-declarative

2011-03-31 Thread Adam Young

On 03/31/2011 12:51 PM, Endi Sukma Dewata wrote:

On 3/31/2011 8:43 AM, Adam Young wrote:

rebased both patches


I don't see any code change in the rebased patches, only new commit 
ID's, I hope this is correct.


Some comments (some of which had been discussed over IRC):

1. I ran my Selenium test cases against 215, 216, and 217 together,
   so far there's no failure.

Good to know.



2. There's a IPA.metadata assignment and the like in unit tests, this
   is redundant.


yeah, I suspect it was from before I explicitly set async.  removed.



3. In IPA.entity_builder.section(), the current_section should be added
   to the current_facet before adding the fields to do incremental
   construction.

Done.



4. In IPA.entity_builder the entity_name can be replaced with
   entity.name to reduce the number of variables.


Done


5. In IPA.entity_builder the standard_associations() can be replaced
   standard_association_facets() for consistency.

Done


6. In the permission entity definition, the 'add_fields' is used
   inconsistently to add a section (i.e. IPA.target_section). The
   solution is either adding 'add_sections' or converting
   IPA.target_sections into widgets. I think adding 'add_sections' is
   simpler because widgets is designed to represent a single attribute.


Gonna punt on this for this patch.  Not certain on the correct approach, 
either to make adders have sections, or to convert the one custom 
section we have to a widget.  Either way, beyond the scope of this 
patch, and it will only affect one entity and the builder when we decide.




7. The IPA.entity_builder.details_facet() takes an array of sections
   instead of a spec object. This limits the expandability of the
   builder interface. It should take a spec object with a 'sections'
   attribute containing the array of sections, this would be consistent
   with the other interfaces.


I thought about it, but there is nothing that we want to customize in 
the details_facet.  We can always do a custom facet to customize.  An 
alternative is to check the type of that is passed in to the 
details_section method on the builder, check if it is an object or an 
array, and treat it like a spec or array depending.




8. In IPA.entity_builder.search_facet(), there's no need to call
   current_facet.init() because all facets will be initialized by
   the entity when IPA.start_entities() is invoked.


Done.



9. The IPA.entity_builder could be a singleton because it doesn't take
   any parameters and there's no multi-threading issue.
Going to leave it like this for now.  That change would be limited to a 
single file (entity.js) and can be done if we decide we want to with a 
very small patch.





10. In IPA.details_refresh() it calls IPA.refresh_devel_hook()
to execute a code specifically used by testing. I think ideally
the develop.js should modify the entity_builder instance to generate
details facet with test-specific code. This requires item #9. But
for now at least we should rename IPA.refresh_devel_hook() into
IPA.details_refresh_devel_hook() for clarity.


Good idea.  Done


 I had already submitted patch freeipa-admiyo-0218-default-all-false.  
Some of the fixes here would conflict with that patch if rebased.  What 
I've attached is on top of 217-2 and 218-1.



From 849e9ea5afaf79fef8d438eb18d788239bda9207 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Thu, 31 Mar 2011 13:57:35 -0400
Subject: [PATCH] code review fixes

---
 install/ui/aci.js|8 
 install/ui/details.js|4 ++--
 install/ui/develop.js|2 +-
 install/ui/dns.js|2 +-
 install/ui/entity.js |   32 ++--
 install/ui/entitylist|   15 +++
 install/ui/group.js  |2 +-
 install/ui/host.js   |2 +-
 install/ui/hostgroup.js  |2 +-
 install/ui/netgroup.js   |2 +-
 install/ui/policy.js |2 +-
 install/ui/service.js|3 ++-
 install/ui/test/details_tests.js |5 -
 install/ui/test/entity_tests.js  |5 -
 install/ui/user.js   |2 +-
 15 files changed, 41 insertions(+), 47 deletions(-)
 create mode 100644 install/ui/entitylist

diff --git a/install/ui/aci.js b/install/ui/aci.js
index d51741245f4305a1bdc87acb5fc639b30a5fd23e..7d669fc9155584849ae74f5d0a9d4ed336779509 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -60,7 +60,7 @@ IPA.entity_factories.permission = function() {
 factory:IPA.target_section,
 label: IPA.messages.objects.permission.target
 }]).
-standard_associations().
+standard_association_facets().
 build();
 };
 
@@ -88,7 +88,7 @@ IPA.entity_factories.privilege = function() {
 add_method: 'add_permission',
 remove_method: 'remove_permission'
 }).
-

Re: [Freeipa-devel] [PATCH] admiyo-0217-define-entities-using-builder-and-more-declarative

2011-03-31 Thread Adam Young

On 03/31/2011 03:12 PM, Adam Young wrote:

On 03/31/2011 02:09 PM, Adam Young wrote:

On 03/31/2011 12:51 PM, Endi Sukma Dewata wrote:

On 3/31/2011 8:43 AM, Adam Young wrote:

rebased both patches


I don't see any code change in the rebased patches, only new commit 
ID's, I hope this is correct.


Some comments (some of which had been discussed over IRC):

1. I ran my Selenium test cases against 215, 216, and 217 together,
   so far there's no failure.

Good to know.



2. There's a IPA.metadata assignment and the like in unit tests, this
   is redundant.


yeah, I suspect it was from before I explicitly set async.  removed.



3. In IPA.entity_builder.section(), the current_section should be added
   to the current_facet before adding the fields to do incremental
   construction.

Done.



4. In IPA.entity_builder the entity_name can be replaced with
   entity.name to reduce the number of variables.


Done


5. In IPA.entity_builder the standard_associations() can be replaced
   standard_association_facets() for consistency.

Done


6. In the permission entity definition, the 'add_fields' is used
   inconsistently to add a section (i.e. IPA.target_section). The
   solution is either adding 'add_sections' or converting
   IPA.target_sections into widgets. I think adding 'add_sections' is
   simpler because widgets is designed to represent a single attribute.


Gonna punt on this for this patch.  Not certain on the correct 
approach, either to make adders have sections, or to convert the one 
custom section we have to a widget.  Either way, beyond the scope of 
this patch, and it will only affect one entity and the builder when 
we decide.




7. The IPA.entity_builder.details_facet() takes an array of sections
   instead of a spec object. This limits the expandability of the
   builder interface. It should take a spec object with a 'sections'
   attribute containing the array of sections, this would be consistent
   with the other interfaces.


I thought about it, but there is nothing that we want to customize in 
the details_facet.  We can always do a custom facet to customize.  An 
alternative is to check the type of that is passed in to the 
details_section method on the builder, check if it is an object or an 
array, and treat it like a spec or array depending.




8. In IPA.entity_builder.search_facet(), there's no need to call
   current_facet.init() because all facets will be initialized by
   the entity when IPA.start_entities() is invoked.


Done.



9. The IPA.entity_builder could be a singleton because it doesn't take
   any parameters and there's no multi-threading issue.
Going to leave it like this for now.  That change would be limited to 
a single file (entity.js) and can be done if we decide we want to 
with a very small patch.





10. In IPA.details_refresh() it calls IPA.refresh_devel_hook()
to execute a code specifically used by testing. I think ideally
the develop.js should modify the entity_builder instance to 
generate

details facet with test-specific code. This requires item #9. But
for now at least we should rename IPA.refresh_devel_hook() into
IPA.details_refresh_devel_hook() for clarity.


Good idea.  Done


 I had already submitted patch 
freeipa-admiyo-0218-default-all-false.  Some of the fixes here would 
conflict with that patch if rebased.  What I've attached is on top of 
217-2 and 218-1.




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

Including Automount


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

Hadn't merged in the changes.  Updated patch attached.
From bfbb7564e318f596dbaec8e29cdc5fb91600f1eb Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Thu, 31 Mar 2011 13:57:35 -0400
Subject: [PATCH] code review fixes

---
 install/ui/aci.js|8 
 install/ui/automount.js  |2 +-
 install/ui/details.js|4 ++--
 install/ui/develop.js|2 +-
 install/ui/dns.js|2 +-
 install/ui/entity.js |   32 ++--
 install/ui/entitylist|   15 +++
 install/ui/group.js  |2 +-
 install/ui/host.js   |2 +-
 install/ui/hostgroup.js  |2 +-
 install/ui/netgroup.js   |2 +-
 install/ui/policy.js |2 +-
 install/ui/service.js|3 ++-
 install/ui/test/details_tests.js |5 -
 install/ui/test/entity_tests.js  |5 -
 install/ui/user.js   |2 +-
 16 files changed, 42 insertions(+), 48 deletions(-)
 create mode 100644 install/ui/entitylist

diff --git a/install/ui/aci.js b/install/ui/aci.js
index d51741245f4305a1bdc87acb5fc639b30a5fd23e..7d669fc9155584849ae74f5d0a9d4ed336779509 100644
--- 

Re: [Freeipa-devel] [PATCH] 5 Add note about ipa-dns-install to ipa-server-install man page

2011-03-31 Thread Rob Crittenden

David O'Brien wrote:

Jan Cholasta wrote:

On 30.3.2011 01:01, David O'Brien wrote:

Jan Cholasta wrote:

Added the note so that users know that they can setup DNS at any time
after ipa-server-install.

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





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

NACK

Minor English and style fix:

s/
Note that you can setup DNS at any later time by running
ipa-dns-install
/
Note that you can set up a DNS at any time after the initial IPA server
install by running ipa-dns-install.


Thanks, fixed.



cheers




ACK



Pushed to master and ipa-2-0

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


Re: [Freeipa-devel] [PATCH] 23 Optimize and dynamically verify group membership

2011-03-31 Thread JR Aquino
Better formatting for the statistics:

-=-

Without patch

ipa hostgroup-find

...

-
Number of entries returned 52
-

real0m20.054s
user0m0.934s
sys 0m0.050s

-=-

With Patch
ipa find-hostgroup

...

-
Number of entries returned 52
-

real0m15.064s
user0m0.945s
sys 0m0.057s

-=-

Without Patch
ipa host-find

...

--
Number of entries returned 100
--

real0m41.277s
user0m0.806s
sys 0m0.060s

-=-

With Patch
ipa host-find

...

--
Number of entries returned 100
--

real0m16.385s
user0m0.814s
sys 0m0.053s

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


Re: [Freeipa-devel] [PATCH] 23 Optimize and dynamically verify group membership

2011-03-31 Thread Rob Crittenden

JR Aquino wrote:

The following patch Removes around 20 lines of code and provides a substantial 
increase in performance for FreeIPA member/memberof verification searches.

The current code base blindly searches static containers for the possible 
presence of members.

This patch provides a method for dynamically identifying the specific objects 
to verify memberships for.

The attached patch addresses ticket:
https://fedorahosted.org/freeipa/ticket/1139

Without patch

ipa hostgroup-find

...

-
Number of entries returned 52
-

real0m20.054s
user0m0.934s
sys 0m0.050s

With Patch
ipa find-hostgroup

...

-
Number of entries returned 52
-

real0m15.064s
user0m0.945s
sys 0m0.057s


--
Number of entries returned 100
--

real0m16.471s
user0m0.814s
sys 0m0.040s

Without Patch
ipa host-find

...

--
Number of entries returned 100
--

real0m41.277s
user0m0.806s
sys 0m0.060s

With Patch
ipa host-find

...

--
Number of entries returned 100
--

real0m16.385s
user0m0.814s
sys 0m0.053s


There is a typo in the first block, memeber.

Wouldn't it be clearer to do a negative test to continue:

if not 'member' in r[1]:
continue

rob

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


Re: [Freeipa-devel] [PATCH] 23 Optimize and dynamically verify group membership

2011-03-31 Thread JR Aquino
On Mar 31, 2011, at 1:48 PM, Rob Crittenden wrote:

 JR Aquino wrote:
 The following patch Removes around 20 lines of code and provides a 
 substantial increase in performance for FreeIPA member/memberof verification 
 searches.
 
 The current code base blindly searches static containers for the possible 
 presence of members.
 
 This patch provides a method for dynamically identifying the specific 
 objects to verify memberships for.
 
 The attached patch addresses ticket:
 https://fedorahosted.org/freeipa/ticket/1139
 
 Without patch
 
 ipa hostgroup-find
 
 ...
 
 -
 Number of entries returned 52
 -
 
 real 0m20.054s
 user 0m0.934s
 sys  0m0.050s
 
 With Patch
 ipa find-hostgroup
 
 ...
 
 -
 Number of entries returned 52
 -
 
 real 0m15.064s
 user 0m0.945s
 sys  0m0.057s
 
 
 --
 Number of entries returned 100
 --
 
 real 0m16.471s
 user 0m0.814s
 sys  0m0.040s
 
 Without Patch
 ipa host-find
 
 ...
 
 --
 Number of entries returned 100
 --
 
 real 0m41.277s
 user 0m0.806s
 sys  0m0.060s
 
 With Patch
 ipa host-find
 
 ...
 
 --
 Number of entries returned 100
 --
 
 real 0m16.385s
 user 0m0.814s
 sys  0m0.053s
 
 There is a typo in the first block, memeber.
 
 Wouldn't it be clearer to do a negative test to continue:
 
 if not 'member' in r[1]:
continue
 
 rob

You're right!

Corrected patch attached.



binL3jhIuM4kH.bin
Description: freeipa-jraquino-0023-Optimize-and-dynamically-verify-group-membership.patch

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

Re: [Freeipa-devel] [PATCH] admiyo-0217-define-entities-using-builder-and-more-declarative

2011-03-31 Thread Adam Young

On 03/31/2011 04:29 PM, Adam Young wrote:

On 03/31/2011 03:27 PM, Adam Young wrote:

On 03/31/2011 03:12 PM, Adam Young wrote:

On 03/31/2011 02:09 PM, Adam Young wrote:

On 03/31/2011 12:51 PM, Endi Sukma Dewata wrote:

On 3/31/2011 8:43 AM, Adam Young wrote:

rebased both patches


I don't see any code change in the rebased patches, only new 
commit ID's, I hope this is correct.


Some comments (some of which had been discussed over IRC):

1. I ran my Selenium test cases against 215, 216, and 217 together,
   so far there's no failure.

Good to know.



2. There's a IPA.metadata assignment and the like in unit tests, this
   is redundant.


yeah, I suspect it was from before I explicitly set async.  removed.



3. In IPA.entity_builder.section(), the current_section should be 
added

   to the current_facet before adding the fields to do incremental
   construction.

Done.



4. In IPA.entity_builder the entity_name can be replaced with
   entity.name to reduce the number of variables.


Done


5. In IPA.entity_builder the standard_associations() can be replaced
   standard_association_facets() for consistency.

Done


6. In the permission entity definition, the 'add_fields' is used
   inconsistently to add a section (i.e. IPA.target_section). The
   solution is either adding 'add_sections' or converting
   IPA.target_sections into widgets. I think adding 'add_sections' is
   simpler because widgets is designed to represent a single 
attribute.


Gonna punt on this for this patch.  Not certain on the correct 
approach, either to make adders have sections, or to convert the 
one custom section we have to a widget.  Either way, beyond the 
scope of this patch, and it will only affect one entity and the 
builder when we decide.




7. The IPA.entity_builder.details_facet() takes an array of sections
   instead of a spec object. This limits the expandability of the
   builder interface. It should take a spec object with a 'sections'
   attribute containing the array of sections, this would be 
consistent

   with the other interfaces.


I thought about it, but there is nothing that we want to customize 
in the details_facet.  We can always do a custom facet to 
customize.  An alternative is to check the type of that is passed 
in to the details_section method on the builder, check if it is an 
object or an array, and treat it like a spec or array depending.




8. In IPA.entity_builder.search_facet(), there's no need to call
   current_facet.init() because all facets will be initialized by
   the entity when IPA.start_entities() is invoked.


Done.



9. The IPA.entity_builder could be a singleton because it doesn't 
take

   any parameters and there's no multi-threading issue.
Going to leave it like this for now.  That change would be limited 
to a single file (entity.js) and can be done if we decide we want 
to with a very small patch.





10. In IPA.details_refresh() it calls IPA.refresh_devel_hook()
to execute a code specifically used by testing. I think ideally
the develop.js should modify the entity_builder instance to 
generate

details facet with test-specific code. This requires item #9. But
for now at least we should rename IPA.refresh_devel_hook() into
IPA.details_refresh_devel_hook() for clarity.


Good idea.  Done


 I had already submitted patch 
freeipa-admiyo-0218-default-all-false.  Some of the fixes here 
would conflict with that patch if rebased.  What I've attached is 
on top of 217-2 and 218-1.




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

Including Automount


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

Hadn't merged in the changes.  Updated patch attached.


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

This version uses a spec for the details facet, IAW code review feedback


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

ACKed after lengthy discussion and minor fix up in IRC

Patches 217 and 219 pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] admiyo-0218-default-all-false.

2011-03-31 Thread Adam Young

On 03/30/2011 09:11 PM, Adam Young wrote:

Requires patch 217


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

rebased, ACKed in IRC and pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] admiyo-0215-Fixed-labels-for-sudo-and-hbac-rules

2011-03-31 Thread Adam Young

On 03/29/2011 05:10 AM, Martin Kosek wrote:

On Mon, 2011-03-28 at 16:49 -0400, Adam Young wrote:

Putting these two patches togetehr because the first changes labels from
the server, and the second is only for test data.  The second is a
separate patch becasue there are other changes from older server side
updates.

Patch 215: ACK

Patch 216: NACK. It breaks the test suite.

Martin

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




Rebased, and ACKed in IRC by edewata.  Both were pushed.  A different 
patch fixed the Unit tests.


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


Re: [Freeipa-devel] [PATCH] 23 Optimize and dynamically verify group membership

2011-03-31 Thread JR Aquino
To clarify, the high delay times in these stats are due to buffered logging 
being turned off.

The ratio of performance increase is ~the same with buffered logging turned on; 
e.g 1.9 seconds down to 1.5 


On Mar 31, 2011, at 1:43 PM, JR Aquino jr.aqu...@citrix.com wrote:

 Better formatting for the statistics:
 
 -=-
 
 Without patch
 
 ipa hostgroup-find
 
 ...
 
 -
 Number of entries returned 52
 -
 
 real0m20.054s
 user0m0.934s
 sys0m0.050s
 
 -=-
 
 With Patch
 ipa find-hostgroup
 
 ...
 
 -
 Number of entries returned 52
 -
 
 real0m15.064s
 user0m0.945s
 sys0m0.057s
 
 -=-
 
 Without Patch
 ipa host-find
 
 ...
 
 --
 Number of entries returned 100
 --
 
 real0m41.277s
 user0m0.806s
 sys0m0.060s
 
 -=-
 
 With Patch
 ipa host-find
 
 ...
 
 --
 Number of entries returned 100
 --
 
 real0m16.385s
 user0m0.814s
 sys0m0.053s
 
 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

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


Re: [Freeipa-devel] [PATCH] Initial Selenium test cases.

2011-03-31 Thread Adam Young

On 03/31/2011 02:09 PM, Endi Sukma Dewata wrote:

On 3/31/2011 12:06 PM, Endi Sukma Dewata wrote:

http://www.freeipa.org/page/Selenium


Patch included.


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

ACK.  Pushed to master.


Note that I did not review each test in depth, but just spot checked.  
If we find that we have regular breakages on certain tests, we will 
investigate those tests more.


Please update the wiki page.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel