Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet
On 4/27/2011 8:47 PM, Adam Young wrote: The more I think about it, the more I think that this structure can be created by the navigation code, and then passed to the entity to populate. The entity-header function moves to navigation.js, but does not have a reference to the entity yet. When an entity tab gets activated, we then populate this structure. everything can get created on demand. Not sure, I'd have to see how it's implemented. My concern is whether it would limit entity customization. I'm assuming that entity-search is for the search bar, and not the search facet: search facet still goes inside the entity-content, right? Yes, it's for the search field. The search facet will use a 'facet' class, same as other facets. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet
On 4/27/2011 8:09 PM, Adam Young wrote:
What do you think? We can do this in a separate patch if it's too
complicated.
Very clean: I like it a lot. I'd like to do it as a separate patch, just
to keep clear what we are doing.
Found some issues:
1. The selector on entity.js:314 is missing a # sign:
var facet_container = $('#'+facet.name +'_container',
entity.header.content);
2. Entity header is not updated after switching facets.
Open Users tab, click one of the users, then click Back to Search.
The title and buttons are wrong and the facet tabs do not disappear.
This is because entity header is shared with other facets and isn't
updated if the facet container already exists.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet
On 04/27/2011 09:09 PM, Adam Young wrote:
On 04/27/2011 04:39 PM, Endi Sukma Dewata wrote:
On 4/27/2011 12:56 PM, Adam Young wrote:
Currently we have this DOM structure:
1. The nested entity-container is confusing and probably will make
CSS adjustment harder. The entity content should be in the same level
as entity header.
2. The _container suffix for the facet container name is unnecessary
because it already has a facet_container class.
3. The use of id attribute could lead to conflicts. It's better to
use a name attribute and a class. Querying the element can be done
with this selector:
$('.facet[name=search]', ...)
4. Rename action-controls to entity-controls for consistency.
5. Rename facet_container to facet-container for consistency.
6. The hidden pkey can be stored in the entity object instead of DOM.
The DOM structure will look like this:
What do you think? We can do this in a separate patch if it's too
complicated.
Very clean: I like it a lot. I'd like to do it as a separate patch,
just to keep clear what we are doing.
The more I think about it, the more I think that this structure can be
created by the navigation code, and then passed to the entity to
populate. The entity-header function moves to navigation.js, but does
not have a reference to the entity yet. When an entity tab gets
activated, we then populate this structure. everything can get created
on demand.
I'm assuming that entity-search is for the search bar, and not the
search facet: search facet still goes inside the entity-content, right?
___
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] admiyo-0226-Added-a-container-for-the-facet
On 04/27/2011 04:39 PM, Endi Sukma Dewata wrote:
On 4/27/2011 12:56 PM, Adam Young wrote:
Currently we have this DOM structure:
1. The nested entity-container is confusing and probably will make CSS
adjustment harder. The entity content should be in the same level as
entity header.
2. The _container suffix for the facet container name is unnecessary
because it already has a facet_container class.
3. The use of id attribute could lead to conflicts. It's better to use
a name attribute and a class. Querying the element can be done with
this selector:
$('.facet[name=search]', ...)
4. Rename action-controls to entity-controls for consistency.
5. Rename facet_container to facet-container for consistency.
6. The hidden pkey can be stored in the entity object instead of DOM.
The DOM structure will look like this:
What do you think? We can do this in a separate patch if it's too
complicated.
Very clean: I like it a lot. I'd like to do it as a separate patch,
just to keep clear what we are doing.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet
On 4/27/2011 12:56 PM, Adam Young wrote:
Currently we have this DOM structure:
1. The nested entity-container is confusing and probably will make CSS
adjustment harder. The entity content should be in the same level as
entity header.
2. The _container suffix for the facet container name is unnecessary
because it already has a facet_container class.
3. The use of id attribute could lead to conflicts. It's better to use a
name attribute and a class. Querying the element can be done with this
selector:
$('.facet[name=search]', ...)
4. Rename action-controls to entity-controls for consistency.
5. Rename facet_container to facet-container for consistency.
6. The hidden pkey can be stored in the entity object instead of DOM.
The DOM structure will look like this:
What do you think? We can do this in a separate patch if it's too
complicated.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet
From f4f5a644ffcc31c417c6d81998b52e2b17c8f0e3 Mon Sep 17 00:00:00 2001
From: Adam Young
Date: Wed, 27 Apr 2011 13:32:14 -0400
Subject: [PATCH] Added a container for the facet
---
install/ui/details.js |2 +-
install/ui/entity.js | 24 ++--
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/install/ui/details.js b/install/ui/details.js
index 1d653c23752e7a4a6a5c87ae3bd5c3379768e05d..877142032c02e59d0d52c8fbe699bb932facb68d 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -573,7 +573,7 @@ IPA.details_refresh = function() {
method: 'show',
options: { all: true, rights: true }
});
-
+
if (IPA.details_refresh_devel_hook){
IPA.details_refresh_devel_hook(that.entity_name,command,that.pkey);
}
diff --git a/install/ui/entity.js b/install/ui/entity.js
index 9a9f05f23476f0b768e3d52d739f790011bdcaa2..ccf41c39eff0fe48fc4b8421c1b2b683f2ab89b0 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -189,7 +189,7 @@ IPA.entity = function (spec) {
facet.entity_name = that.name;
that.facets.push(facet);
that.facets_by_name[facet.name] = facet;
-
+
if (facet.facet_group){
if (!that.facet_groups[facet.facet_group]){
that.facet_groups[facet.facet_group] = [];
@@ -309,9 +309,21 @@ IPA.entity_setup = function (container) {
}
facet.entity_header = entity.header;
-entity.header.reset();
-facet.create_content(facet.entity_header.content);
-facet.setup(facet.entity_header.content);
+$('.facet_container',entity.header.content).css('display','none');
+
+var facet_container = $(facet.name +'_container', entity.header.content);
+if (!facet_container.length){
+facet_container = $('',{
+id:facet.name +'_container',
+'class':'facet_container'
+}).appendTo(facet.entity_header.content);
+entity.header.reset();
+facet.create_content(facet_container);
+facet.setup(facet_container);
+}else{
+facet_container.css('display','block');
+}
+
entity.header.select_tab();
facet.refresh();
};
@@ -532,7 +544,8 @@ IPA.entity_header = function(spec){
return that.facet_tabs;
}
function content(){
-that.content = $("");return that.content;
+that.content = $("");
+return that.content;
}
function entity_container() {
@@ -548,7 +561,6 @@ IPA.entity_header = function(spec){
function reset(){
that.buttons.empty();
-that.content.empty();
}
that.reset = reset;
--
1.7.4.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 04/27/2011 12:33 PM, Adam Young wrote: > On 04/27/2011 10:24 AM, Martin Kosek wrote: >> On Wed, 2011-04-27 at 09:56 -0400, Adam Young wrote: >>> On 04/27/2011 09:34 AM, Dmitri Pal wrote: On 04/27/2011 08:13 AM, Jan Cholasta wrote: > On 27.4.2011 13:17, Martin Kosek wrote: >> On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: >>> On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: > Automatically run the lint script during make > rpms|client-rpms|srpms. > NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. >>> I think printing a note asking the developer to kindly install the >>> missing packages would be sufficient. AFAIK there are currently >>> only 2 >>> optional packages: python-rhsm and python-krbV. python-krbV is >>> optional >>> only for the client part of IPA, so you most likely have it already >>> installed and installing python-rhsm is not really much of a chore. >>> That >>> way all of the code would always be checked and the lint script >>> would be >>> free of the unnecessary complexity of handling missing packages. >> I don't think this is a right approach. When the package is optional >> (currently it may be python-rhsm and python-krbV only, but there >> may be >> others in the future) I shouldn't be obliged to install them in >> order to >> build IPA. > You shouldn't be obliged to install them as a user. As a developer, > you should be ready for all kinds of crazy stuff IMHO. > >> When somebody develops something related with the optional >> package he has them installed and the lint will check the >> relevant code >> too. > All of the code goes to the package, so it all should be checked > during the build. > > Imagine situation like this: You change something in module A, > accidentally breaking functionality that module B depends on. > Module A > is checked and no error is found (because it's the kind of issue that > exhibits itself only in certain conditions). Module B isn't checked, > because it also depends on a not-installed optional package. If it > was > checked, it would report an error that would lead you to the error in > module A. But everything looks fine, so the build succeeds, even when > the error is there. > > The situation might be improbable, but IMO the code should be checked > in the same ecosystem every time anyway, because weird stuff could > happen if it wasn't. > >> It is not that big deal, I just think it would be an annoyance for >> developers. But maybe there is a different opinion. > I know we developers are lazy folk, but this is a matter of writing > one simple command, just one time. > >> Martin >> How about a compromise? By default everything is expected to be installed. But there is a command line switch that allows to skip modules you want to skip. You provide the switch and the list of the modules to skip and build will validate only modules that are not in the list. Will something like this work? >>> Actually, make the command line switch just means that a Lint failure >>> doesn't stop the build. That way, by default the build will fail >>> unless >>> everything is there and checked, but there is a way to move forward for >>> building with a subset of packages. >> Yes, I think we will can settle with a compromise. My only concern was >> not to force the developers to install unnecessary packages for build. >> >> I would suggest that the build (or "make lint") succeeds without those >> optional packages installed, but a warning is printed out that some >> packages are missing and not all the code is checked. >> >> Then it is a developers responsibility to handle this and wouldn't be >> force to install those packages for his test builds etc. > > How about instead it fails bny default, but prints the message "to > suppress the lint check stopping the build, run make > --no-fail-on-lint" so that skipping lint is a deliberate decision? Yes this is the approach I prefer. > >> Martin >> >> ___ >> 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 > > -- Thank you, Dmitri Pal Sr. Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts
Re: [Freeipa-devel] [PATCH] fixes #1193 init script related
Simo Sorce wrote: On Wed, 2011-04-27 at 10:59 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Tue, 2011-04-26 at 17:08 -0400, Rob Crittenden wrote: Simo Sorce wrote: With this patch we stop clearing the environment variables when running binaries allowing for env variables to be passed down by default. This fixes also the init script against Fedora 15 and systemd Simo. I think it would be better to import copy and use: if env is None: env = copy.deepcopy(os.environ) env["PATH"] = "/bin:/sbin:..." rob Updated patch attached. Simo. ack Pushed to master. Simo. Pushed to ipa-2-0 branch rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 04/27/2011 10:24 AM, Martin Kosek wrote: On Wed, 2011-04-27 at 09:56 -0400, Adam Young wrote: On 04/27/2011 09:34 AM, Dmitri Pal wrote: On 04/27/2011 08:13 AM, Jan Cholasta wrote: On 27.4.2011 13:17, Martin Kosek wrote: On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: Automatically run the lint script during make rpms|client-rpms|srpms. NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. I think printing a note asking the developer to kindly install the missing packages would be sufficient. AFAIK there are currently only 2 optional packages: python-rhsm and python-krbV. python-krbV is optional only for the client part of IPA, so you most likely have it already installed and installing python-rhsm is not really much of a chore. That way all of the code would always be checked and the lint script would be free of the unnecessary complexity of handling missing packages. I don't think this is a right approach. When the package is optional (currently it may be python-rhsm and python-krbV only, but there may be others in the future) I shouldn't be obliged to install them in order to build IPA. You shouldn't be obliged to install them as a user. As a developer, you should be ready for all kinds of crazy stuff IMHO. When somebody develops something related with the optional package he has them installed and the lint will check the relevant code too. All of the code goes to the package, so it all should be checked during the build. Imagine situation like this: You change something in module A, accidentally breaking functionality that module B depends on. Module A is checked and no error is found (because it's the kind of issue that exhibits itself only in certain conditions). Module B isn't checked, because it also depends on a not-installed optional package. If it was checked, it would report an error that would lead you to the error in module A. But everything looks fine, so the build succeeds, even when the error is there. The situation might be improbable, but IMO the code should be checked in the same ecosystem every time anyway, because weird stuff could happen if it wasn't. It is not that big deal, I just think it would be an annoyance for developers. But maybe there is a different opinion. I know we developers are lazy folk, but this is a matter of writing one simple command, just one time. Martin How about a compromise? By default everything is expected to be installed. But there is a command line switch that allows to skip modules you want to skip. You provide the switch and the list of the modules to skip and build will validate only modules that are not in the list. Will something like this work? Actually, make the command line switch just means that a Lint failure doesn't stop the build. That way, by default the build will fail unless everything is there and checked, but there is a way to move forward for building with a subset of packages. Yes, I think we will can settle with a compromise. My only concern was not to force the developers to install unnecessary packages for build. I would suggest that the build (or "make lint") succeeds without those optional packages installed, but a warning is printed out that some packages are missing and not all the code is checked. Then it is a developers responsibility to handle this and wouldn't be force to install those packages for his test builds etc. How about instead it fails bny default, but prints the message "to suppress the lint check stopping the build, run make --no-fail-on-lint" so that skipping lint is a deliberate decision? Martin ___ 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] 143 Entitlement quantity validation.
On 4/27/2011 10:43 AM, Adam Young wrote: The widget base class has been modified to validate integer value if the type is specified in the metadata. This is used to validate entitlement quantity. Logic looks good. I'd like to make the validator an external object, and have the builder select the validator based on the metadata. I had a similar thought, but the validator interface needs further thinking. I'll keep the patch as is for now. This can be refactored into external validator in a follow on patch. Sounds good. ACK Pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 143 Entitlement quantity validation.
On 04/27/2011 10:34 AM, Endi Sukma Dewata wrote:
On 4/26/2011 8:23 PM, Adam Young wrote:
The widget base class has been modified to validate integer value
if the type is specified in the metadata. This is used to validate
entitlement quantity.
Logic looks good. I'd like to make the validator an external object, and
have the builder select the validator based on the metadata.
The IPA data types beyond String and int are:
bool, float, bytes, and enum. We should at least plan for them, even if
we are not implementing them now:
something like
var validators = {
int: function (blah){...}
string: function (regex){}
}
widget.spec. validator = validators[type];
I had a similar thought, but the validator interface needs further
thinking. I'll keep the patch as is for now. This can be refactored
into external validator in a follow on patch.
Sounds good. ACK
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] fixes #1193 init script related
On Wed, 2011-04-27 at 10:59 -0400, Rob Crittenden wrote: > Simo Sorce wrote: > > On Tue, 2011-04-26 at 17:08 -0400, Rob Crittenden wrote: > >> Simo Sorce wrote: > >>> With this patch we stop clearing the environment variables when running > >>> binaries allowing for env variables to be passed down by default. > >>> > >>> This fixes also the init script against Fedora 15 and systemd > >>> > >>> Simo. > >>> > >> > >> I think it would be better to import copy and use: > >> > >> if env is None: > >> env = copy.deepcopy(os.environ) > >> env["PATH"] = "/bin:/sbin:..." > >> > >> rob > > > > Updated patch attached. > > > > Simo. > > > > ack Pushed to master. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 780 Handle principal not found errors when converting replication agreements
There are times where one side or the other is missing its peers krbprincipalname when converting from simple to GSSAPI replication. Ticket 1188 should address the cause of this. This patch provides better information and handling should either side be missing. ticket 1044 rob freeipa-rcrit-780-replica.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] fixes #1193 init script related
Simo Sorce wrote: On Tue, 2011-04-26 at 17:08 -0400, Rob Crittenden wrote: Simo Sorce wrote: With this patch we stop clearing the environment variables when running binaries allowing for env variables to be passed down by default. This fixes also the init script against Fedora 15 and systemd Simo. I think it would be better to import copy and use: if env is None: env = copy.deepcopy(os.environ) env["PATH"] = "/bin:/sbin:..." rob Updated patch attached. Simo. ack ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 143 Entitlement quantity validation.
On 04/27/2011 10:34 AM, Endi Sukma Dewata wrote:
> On 4/26/2011 8:23 PM, Adam Young wrote:
>>> The widget base class has been modified to validate integer value
>>> if the type is specified in the metadata. This is used to validate
>>> entitlement quantity.
>
>> Logic looks good. I'd like to make the validator an external object, and
>> have the builder select the validator based on the metadata.
>>
>> The IPA data types beyond String and int are:
>>
>> bool, float, bytes, and enum. We should at least plan for them, even if
>> we are not implementing them now:
>>
>> something like
>>
>> var validators = {
>> int: function (blah){...}
>> string: function (regex){}
>> }
>>
>> widget.spec. validator = validators[type];
>
> I had a similar thought, but the validator interface needs further
> thinking. I'll keep the patch as is for now. This can be refactored
> into external validator in a follow on patch.
>
We will have a conversation about the entitlements next week.
We need to get on the same page with the entitlements team and
straighten the use cases.
--
Thank you,
Dmitri Pal
Sr. Engineering Manager IPA project,
Red Hat Inc.
---
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 053 Forbid reinstallation in ipa-client-install
On Wed, 2011-04-27 at 16:17 +0200, Martin Kosek wrote: > The --force option may be misused to reinstall an existing IPA > client. This is not supported and may lead to unexpected errors. > When required, the cleanest way to re-install IPA client is to > run uninstall and then install again. > > This patch also includes few cosmetic changes in "LDAP" term case > to provide more consistent experience with the script. > > https://fedorahosted.org/freeipa/ticket/1117 Code looks good, but can you add to the last message advice about running --uninstall in case that's what the user is trying to do (a re-install) Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0225-remove-jquery-cookie-library
On 4/26/2011 7:33 PM, Adam Young wrote: ACK and pushed to master. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 052 Log temporary files in ipa-client-install
On Wed, 2011-04-27 at 13:29 +0200, Martin Kosek wrote: > This patch will help investigating issues like in ticket 1100 resolved > by my patch 50. > > -- > > This patch adds logging of temporary files (Kerberos configuration, > nsupdate commands) that may be very useful for debugging purposes. > > https://fedorahosted.org/freeipa/ticket/1093 > https://fedorahosted.org/freeipa/ticket/1094 Ack, very useuful stuff, thanks! Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 051 Improve Directory Service open port checker
On Wed, 2011-04-27 at 12:43 +0200, Martin Kosek wrote: > Wait for DS ports to open after _every_ DS service restart. > Several restarts were missed by the current open port checker > implementation. > > https://fedorahosted.org/freeipa/ticket/1182 Ack Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 049 IPA replica is not started after the reboot
On Tue, 2011-04-26 at 10:41 +0200, Martin Kosek wrote: > https://fedorahosted.org/freeipa/ticket/1191 > ACK! Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 143 Entitlement quantity validation.
On 4/26/2011 8:23 PM, Adam Young wrote:
The widget base class has been modified to validate integer value
if the type is specified in the metadata. This is used to validate
entitlement quantity.
Logic looks good. I'd like to make the validator an external object, and
have the builder select the validator based on the metadata.
The IPA data types beyond String and int are:
bool, float, bytes, and enum. We should at least plan for them, even if
we are not implementing them now:
something like
var validators = {
int: function (blah){...}
string: function (regex){}
}
widget.spec. validator = validators[type];
I had a similar thought, but the validator interface needs further
thinking. I'll keep the patch as is for now. This can be refactored into
external validator in a follow on patch.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On Wed, 2011-04-27 at 09:56 -0400, Adam Young wrote: > On 04/27/2011 09:34 AM, Dmitri Pal wrote: > > On 04/27/2011 08:13 AM, Jan Cholasta wrote: > >> On 27.4.2011 13:17, Martin Kosek wrote: > >>> On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: > On 26.4.2011 18:14, Martin Kosek wrote: > > On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: > >> Automatically run the lint script during make rpms|client-rpms|srpms. > >> > > NACK until ticket 1184 is resolved and pushed. Currently, pylint check > > fails when optional python packages (like python-rhsm) are not > > installed > > on the machine. We should be able to build IPA without those packages > > installed. > I think printing a note asking the developer to kindly install the > missing packages would be sufficient. AFAIK there are currently only 2 > optional packages: python-rhsm and python-krbV. python-krbV is optional > only for the client part of IPA, so you most likely have it already > installed and installing python-rhsm is not really much of a chore. > That > way all of the code would always be checked and the lint script > would be > free of the unnecessary complexity of handling missing packages. > >>> I don't think this is a right approach. When the package is optional > >>> (currently it may be python-rhsm and python-krbV only, but there may be > >>> others in the future) I shouldn't be obliged to install them in order to > >>> build IPA. > >> You shouldn't be obliged to install them as a user. As a developer, > >> you should be ready for all kinds of crazy stuff IMHO. > >> > >>> When somebody develops something related with the optional > >>> package he has them installed and the lint will check the relevant code > >>> too. > >> All of the code goes to the package, so it all should be checked > >> during the build. > >> > >> Imagine situation like this: You change something in module A, > >> accidentally breaking functionality that module B depends on. Module A > >> is checked and no error is found (because it's the kind of issue that > >> exhibits itself only in certain conditions). Module B isn't checked, > >> because it also depends on a not-installed optional package. If it was > >> checked, it would report an error that would lead you to the error in > >> module A. But everything looks fine, so the build succeeds, even when > >> the error is there. > >> > >> The situation might be improbable, but IMO the code should be checked > >> in the same ecosystem every time anyway, because weird stuff could > >> happen if it wasn't. > >> > >>> It is not that big deal, I just think it would be an annoyance for > >>> developers. But maybe there is a different opinion. > >> I know we developers are lazy folk, but this is a matter of writing > >> one simple command, just one time. > >> > >>> Martin > >>> > >> > > How about a compromise? > > By default everything is expected to be installed. > > But there is a command line switch that allows to skip modules you want > > to skip. You provide the switch and the list of the modules to skip and > > build will validate only modules that are not in the list. > > > > Will something like this work? > > > Actually, make the command line switch just means that a Lint failure > doesn't stop the build. That way, by default the build will fail unless > everything is there and checked, but there is a way to move forward for > building with a subset of packages. Yes, I think we will can settle with a compromise. My only concern was not to force the developers to install unnecessary packages for build. I would suggest that the build (or "make lint") succeeds without those optional packages installed, but a warning is printed out that some packages are missing and not all the code is checked. Then it is a developers responsibility to handle this and wouldn't be force to install those packages for his test builds etc. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 053 Forbid reinstallation in ipa-client-install
The --force option may be misused to reinstall an existing IPA
client. This is not supported and may lead to unexpected errors.
When required, the cleanest way to re-install IPA client is to
run uninstall and then install again.
This patch also includes few cosmetic changes in "LDAP" term case
to provide more consistent experience with the script.
https://fedorahosted.org/freeipa/ticket/1117
>From e12f2a27f6c5f1be26fb26ed307130e2cf6838e8 Mon Sep 17 00:00:00 2001
From: Martin Kosek
Date: Wed, 27 Apr 2011 16:09:43 +0200
Subject: [PATCH] Forbid reinstallation in ipa-client-install
The --force option may be misused to reinstall an existing IPA
client. This is not supported and may lead to unexpected errors.
When required, the cleanest way to re-install IPA client is to
run uninstall and then install again.
This patch also includes few cosmetic changes in "LDAP" term case
to provide more consistent experience with the script.
https://fedorahosted.org/freeipa/ticket/1117
---
ipa-client/ipa-install/ipa-client-install | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index ca96e86f53de5f9b6d22731544c00b357eec2f4e..476afd00dac642a448c9c2064514fecbae9f31de 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -58,7 +58,7 @@ def parse_options():
parser.add_option("--server", dest="server", help="IPA server")
parser.add_option("--realm", dest="realm_name", help="realm name")
parser.add_option("-f", "--force", dest="force", action="store_true",
- default=False, help="force setting of ldap/kerberos conf")
+ default=False, help="force setting of LDAP/Kerberos conf")
parser.add_option("-d", "--debug", dest="debug", action="store_true",
default=False, help="print debugging information")
parser.add_option("-U", "--unattended", dest="unattended",
@@ -185,7 +185,7 @@ def chkconfig(name, status):
def uninstall(options, env):
-if not fstore.has_files() and not options.force:
+if not fstore.has_files():
print "IPA client is not configured on this system."
return 2
@@ -265,11 +265,11 @@ def uninstall(options, env):
except:
print "Failed to clean up /etc/krb5.keytab"
-print "Disabling client Kerberos and Ldap configurations"
+print "Disabling client Kerberos and LDAP configurations"
try:
run(["/usr/sbin/authconfig", "--disableldap", "--disablekrb5", "--disablesssd", "--disablesssdauth", "--disablemkhomedir", "--update"])
except Exception, e:
-print "Failed to remove krb5/ldap configuration. " +str(e)
+print "Failed to remove krb5/LDAP configuration. " +str(e)
sys.exit(1)
print "Restoring client configuration files"
@@ -563,7 +563,7 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options):
return 0
def resolve_ipaddress(server):
-""" Connect to the server's ldap port in order to determine what ip
+""" Connect to the server's LDAP port in order to determine what ip
address this machine uses as "public" ip (relative to the server).
"""
@@ -672,7 +672,7 @@ def main():
if options.uninstall:
return uninstall(options, env)
-if fstore.has_files() and not options.force:
+if fstore.has_files():
sys.exit("IPA client is already configured on this system.")
cli_domain = None
--
1.7.4.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 04/27/2011 09:34 AM, Dmitri Pal wrote: On 04/27/2011 08:13 AM, Jan Cholasta wrote: On 27.4.2011 13:17, Martin Kosek wrote: On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: Automatically run the lint script during make rpms|client-rpms|srpms. NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. I think printing a note asking the developer to kindly install the missing packages would be sufficient. AFAIK there are currently only 2 optional packages: python-rhsm and python-krbV. python-krbV is optional only for the client part of IPA, so you most likely have it already installed and installing python-rhsm is not really much of a chore. That way all of the code would always be checked and the lint script would be free of the unnecessary complexity of handling missing packages. I don't think this is a right approach. When the package is optional (currently it may be python-rhsm and python-krbV only, but there may be others in the future) I shouldn't be obliged to install them in order to build IPA. You shouldn't be obliged to install them as a user. As a developer, you should be ready for all kinds of crazy stuff IMHO. When somebody develops something related with the optional package he has them installed and the lint will check the relevant code too. All of the code goes to the package, so it all should be checked during the build. Imagine situation like this: You change something in module A, accidentally breaking functionality that module B depends on. Module A is checked and no error is found (because it's the kind of issue that exhibits itself only in certain conditions). Module B isn't checked, because it also depends on a not-installed optional package. If it was checked, it would report an error that would lead you to the error in module A. But everything looks fine, so the build succeeds, even when the error is there. The situation might be improbable, but IMO the code should be checked in the same ecosystem every time anyway, because weird stuff could happen if it wasn't. It is not that big deal, I just think it would be an annoyance for developers. But maybe there is a different opinion. I know we developers are lazy folk, but this is a matter of writing one simple command, just one time. Martin How about a compromise? By default everything is expected to be installed. But there is a command line switch that allows to skip modules you want to skip. You provide the switch and the list of the modules to skip and build will validate only modules that are not in the list. Will something like this work? Actually, make the command line switch just means that a Lint failure doesn't stop the build. That way, by default the build will fail unless everything is there and checked, but there is a way to move forward for building with a subset of packages. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 04/27/2011 08:13 AM, Jan Cholasta wrote: > On 27.4.2011 13:17, Martin Kosek wrote: >> On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: >>> On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: > Automatically run the lint script during make rpms|client-rpms|srpms. > NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. >>> >>> I think printing a note asking the developer to kindly install the >>> missing packages would be sufficient. AFAIK there are currently only 2 >>> optional packages: python-rhsm and python-krbV. python-krbV is optional >>> only for the client part of IPA, so you most likely have it already >>> installed and installing python-rhsm is not really much of a chore. >>> That >>> way all of the code would always be checked and the lint script >>> would be >>> free of the unnecessary complexity of handling missing packages. >> >> I don't think this is a right approach. When the package is optional >> (currently it may be python-rhsm and python-krbV only, but there may be >> others in the future) I shouldn't be obliged to install them in order to >> build IPA. > > You shouldn't be obliged to install them as a user. As a developer, > you should be ready for all kinds of crazy stuff IMHO. > >> When somebody develops something related with the optional >> package he has them installed and the lint will check the relevant code >> too. > > All of the code goes to the package, so it all should be checked > during the build. > > Imagine situation like this: You change something in module A, > accidentally breaking functionality that module B depends on. Module A > is checked and no error is found (because it's the kind of issue that > exhibits itself only in certain conditions). Module B isn't checked, > because it also depends on a not-installed optional package. If it was > checked, it would report an error that would lead you to the error in > module A. But everything looks fine, so the build succeeds, even when > the error is there. > > The situation might be improbable, but IMO the code should be checked > in the same ecosystem every time anyway, because weird stuff could > happen if it wasn't. > >> >> It is not that big deal, I just think it would be an annoyance for >> developers. But maybe there is a different opinion. > > I know we developers are lazy folk, but this is a matter of writing > one simple command, just one time. > >> >> Martin >> > > How about a compromise? By default everything is expected to be installed. But there is a command line switch that allows to skip modules you want to skip. You provide the switch and the list of the modules to skip and build will validate only modules that are not in the list. Will something like this work? -- Thank you, Dmitri Pal Sr. Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 27.4.2011 13:17, Martin Kosek wrote: On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: Automatically run the lint script during make rpms|client-rpms|srpms. NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. I think printing a note asking the developer to kindly install the missing packages would be sufficient. AFAIK there are currently only 2 optional packages: python-rhsm and python-krbV. python-krbV is optional only for the client part of IPA, so you most likely have it already installed and installing python-rhsm is not really much of a chore. That way all of the code would always be checked and the lint script would be free of the unnecessary complexity of handling missing packages. I don't think this is a right approach. When the package is optional (currently it may be python-rhsm and python-krbV only, but there may be others in the future) I shouldn't be obliged to install them in order to build IPA. You shouldn't be obliged to install them as a user. As a developer, you should be ready for all kinds of crazy stuff IMHO. When somebody develops something related with the optional package he has them installed and the lint will check the relevant code too. All of the code goes to the package, so it all should be checked during the build. Imagine situation like this: You change something in module A, accidentally breaking functionality that module B depends on. Module A is checked and no error is found (because it's the kind of issue that exhibits itself only in certain conditions). Module B isn't checked, because it also depends on a not-installed optional package. If it was checked, it would report an error that would lead you to the error in module A. But everything looks fine, so the build succeeds, even when the error is there. The situation might be improbable, but IMO the code should be checked in the same ecosystem every time anyway, because weird stuff could happen if it wasn't. It is not that big deal, I just think it would be an annoyance for developers. But maybe there is a different opinion. I know we developers are lazy folk, but this is a matter of writing one simple command, just one time. Martin -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 052 Log temporary files in ipa-client-install
This patch will help investigating issues like in ticket 1100 resolved
by my patch 50.
--
This patch adds logging of temporary files (Kerberos configuration,
nsupdate commands) that may be very useful for debugging purposes.
https://fedorahosted.org/freeipa/ticket/1093
https://fedorahosted.org/freeipa/ticket/1094
>From 2a409db5cc6eacb0ae5381176a7cd5cb10270f62 Mon Sep 17 00:00:00 2001
From: Martin Kosek
Date: Wed, 23 Mar 2011 14:49:48 +0100
Subject: [PATCH] Log temporary files in ipa-client-install
This patch adds logging of temporary files (Kerberos configuration,
nsupdate commands) that may be very useful for debugging purposes.
https://fedorahosted.org/freeipa/ticket/1093
https://fedorahosted.org/freeipa/ticket/1094
---
ipa-client/ipa-install/ipa-client-install |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index ca96e86f53de5f9b6d22731544c00b357eec2f4e..c409fe7062dc09520f45a2103ed883dfad37363e 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -477,6 +477,9 @@ def configure_krb5_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, d
appopts = [{'name':'pam', 'type':'subsection', 'value':pamopts}]
opts.append({'name':'appdefaults', 'type':'section', 'value':appopts})
+logging.debug("Writing Kerberos configuration to %s:\n%s"
+% (filename, krbconf.dump(opts)))
+
krbconf.newConf(filename, opts);
return 0
@@ -616,6 +619,10 @@ def update_dns(server, hostname):
return
update_txt = ipautil.template_str(template, sub_dict)
+
+logging.debug("Writing nsupdate commands to %s:\n%s"
+% (UPDATE_FILE, update_txt))
+
update_fd = file(UPDATE_FILE, "w")
update_fd.write(update_txt)
update_fd.flush()
@@ -628,7 +635,7 @@ def update_dns(server, hostname):
print >>sys.stderr, "Failed to obtain host TGT."
try:
-ipautil.run(['/usr/bin/nsupdate', '-g', "/etc/ipa/.dns_update.txt"],
+ipautil.run(['/usr/bin/nsupdate', '-g', UPDATE_FILE],
env={'KRB5CCNAME':CCACHE_FILE})
print "DNS server record set to: %s -> %s" % (hostname, ip)
except CalledProcessError, e:
--
1.7.4.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On Wed, 2011-04-27 at 12:40 +0200, Jan Cholasta wrote: > On 26.4.2011 18:14, Martin Kosek wrote: > > On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: > >> Automatically run the lint script during make rpms|client-rpms|srpms. > >> > > > > NACK until ticket 1184 is resolved and pushed. Currently, pylint check > > fails when optional python packages (like python-rhsm) are not installed > > on the machine. We should be able to build IPA without those packages > > installed. > > I think printing a note asking the developer to kindly install the > missing packages would be sufficient. AFAIK there are currently only 2 > optional packages: python-rhsm and python-krbV. python-krbV is optional > only for the client part of IPA, so you most likely have it already > installed and installing python-rhsm is not really much of a chore. That > way all of the code would always be checked and the lint script would be > free of the unnecessary complexity of handling missing packages. I don't think this is a right approach. When the package is optional (currently it may be python-rhsm and python-krbV only, but there may be others in the future) I shouldn't be obliged to install them in order to build IPA. When somebody develops something related with the optional package he has them installed and the lint will check the relevant code too. It is not that big deal, I just think it would be an annoyance for developers. But maybe there is a different opinion. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 051 Improve Directory Service open port checker
Wait for DS ports to open after _every_ DS service restart.
Several restarts were missed by the current open port checker
implementation.
https://fedorahosted.org/freeipa/ticket/1182
>From c76be0cf9cb5c1feb58c18e3d7a6bd72594947e4 Mon Sep 17 00:00:00 2001
From: Martin Kosek
Date: Wed, 27 Apr 2011 12:37:04 +0200
Subject: [PATCH] Improve Directory Service open port checker
Wait for DS ports to open after _every_ DS service restart.
Several restarts were missed by the current open port checker
implementation.
https://fedorahosted.org/freeipa/ticket/1182
---
ipaserver/install/dsinstance.py | 21 -
ipaserver/install/service.py|2 +-
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 38195c72cf5337178fe314d6236dd803040ccf79..74243cfc1b4bdda853e03130f01094362fd3bf0a 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -183,6 +183,7 @@ class DsInstance(service.Service):
self.idstart = None
self.idmax = None
self.subject_base = None
+self.open_ports = []
if realm_name:
self.suffix = util.realm_to_suffix(self.realm_name)
self.__setup_sub_dict()
@@ -376,9 +377,13 @@ class DsInstance(service.Service):
logging.debug("completed creating ds instance")
except ipautil.CalledProcessError, e:
logging.critical("failed to restart ds instance %s" % e)
+
+# check for open port 389 from now on
+self.open_ports.append(389)
+
logging.debug("restarting ds instance")
try:
-self.restart(self.serverid)
+self.__restart_instance()
logging.debug("done restarting ds instance")
except ipautil.CalledProcessError, e:
print "failed to restart ds instance", e
@@ -406,18 +411,21 @@ class DsInstance(service.Service):
# Does not apply with newer DS releases
pass
-def __restart_instance(self):
+def restart(self, instance=''):
try:
-self.restart(self.serverid)
+super(DsInstance, self).restart(instance)
if not is_ds_running():
logging.critical("Failed to restart the directory server. See the installation log for details.")
sys.exit(1)
-installutils.wait_for_open_ports('localhost', [389, 636], 300)
+installutils.wait_for_open_ports('localhost', self.open_ports, 300)
except SystemExit, e:
raise e
except Exception, e:
# TODO: roll back here?
-logging.critical("Failed to restart the directory server. See the installation log for details.")
+logging.critical("Failed to restart the directory server (%s). See the installation log for details." % e)
+
+def __restart_instance(self):
+self.restart(self.serverid)
def __enable_entryusn(self):
self._ldap_mod("entryusn.ldif")
@@ -549,6 +557,9 @@ class DsInstance(service.Service):
conn.unbind()
+# check for open secure port 636 from now on
+self.open_ports.append(636)
+
def __add_default_layout(self):
self._ldap_mod("bootstrap-template.ldif", self.sub_dict)
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 0b9bb01ebc44ecdb2423c342f1d7b6ee0d4766c9..1ebd96d7b48a8d8d1df41950da517e18383f2d9c 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -98,7 +98,7 @@ def print_msg(message, output_fd=sys.stdout):
output_fd.write("\n")
-class Service:
+class Service(object):
def __init__(self, service_name, sstore=None, dm_password=None):
self.service_name = service_name
self.steps = []
--
1.7.4.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 14 Run lint during each build
On 26.4.2011 18:14, Martin Kosek wrote: On Tue, 2011-04-26 at 13:42 +0200, Jan Cholasta wrote: Automatically run the lint script during make rpms|client-rpms|srpms. NACK until ticket 1184 is resolved and pushed. Currently, pylint check fails when optional python packages (like python-rhsm) are not installed on the machine. We should be able to build IPA without those packages installed. I think printing a note asking the developer to kindly install the missing packages would be sufficient. AFAIK there are currently only 2 optional packages: python-rhsm and python-krbV. python-krbV is optional only for the client part of IPA, so you most likely have it already installed and installing python-rhsm is not really much of a chore. That way all of the code would always be checked and the lint script would be free of the unnecessary complexity of handling missing packages. Martin -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
