Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-14 Thread Jakub Hrozek
On Fri, Nov 13, 2015 at 09:42:15PM +0200, Nikolai Kondrashov wrote: > On 11/13/2015 07:06 PM, Pavel Reichl wrote: > >On 11/13/2015 05:44 PM, Nikolai Kondrashov wrote: > >>On 11/13/2015 06:19 PM, Pavel Reichl wrote: > >>>Thanks Nick, updated patch attached. > >> > >>Thanks, Pavel, this looks really

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Pavel Reichl
On 11/13/2015 05:44 PM, Nikolai Kondrashov wrote: On 11/13/2015 06:19 PM, Pavel Reichl wrote: Thanks Nick, updated patch attached. Thanks, Pavel, this looks really neat now! I have only two comments. +def test_simple_user_override(ldap_conn, env_simple_user_override): +"""Test entries

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Pavel Reichl
Thanks Nick, updated patch attached. >From ef503789ba6eb8618efef600e063e8031afcb0bc Mon Sep 17 00:00:00 2001 From: Pavel Reichl Date: Mon, 5 Oct 2015 10:02:05 -0400 Subject: [PATCH] intg: Add test for user and group local overrides Introduce a new integration test for local

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Nikolai Kondrashov
On 11/13/2015 06:19 PM, Pavel Reichl wrote: Thanks Nick, updated patch attached. Thanks, Pavel, this looks really neat now! I have only two comments. +def test_simple_user_override(ldap_conn, env_simple_user_override): +"""Test entries are overriden""" + + Shouldn't there be an actual

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Nikolai Kondrashov
On 11/13/2015 07:06 PM, Pavel Reichl wrote: On 11/13/2015 05:44 PM, Nikolai Kondrashov wrote: On 11/13/2015 06:19 PM, Pavel Reichl wrote: Thanks Nick, updated patch attached. Thanks, Pavel, this looks really neat now! I have only two comments. +def test_simple_user_override(ldap_conn,

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Nikolai Kondrashov
Hi Pavel, On 11/13/2015 02:22 PM, Pavel Reichl wrote: Hello Nick, thanks for the valid comments I hope I addressed them all. I used the dictionary hint, but I didn't use it everywhere - mostly just where the same dict would be reused in the same function. Sure, thank you. On 11/03/2015

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-13 Thread Pavel Reichl
Hello Nick, thanks for the valid comments I hope I addressed them all. I used the dictionary hint, but I didn't use it everywhere - mostly just where the same dict would be reused in the same function. On 11/03/2015 04:40 PM, Nikolai Kondrashov wrote: + +# We can create override with

Re: [SSSD] [PATCHSET] intg: sss_override

2015-11-03 Thread Nikolai Kondrashov
Hi Pavel, On 10/30/2015 08:17 PM, Pavel Reichl wrote: attached patch adds tests for {grop,user}-{find,show} commands. I like the overall design much better now. Thanks for working on this Pavel! Please see my comments below. +LDAP_BASE_DN = "dc=example,dc=com" Could you please remove

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-30 Thread Pavel Reichl
Hello, attached patch adds tests for {grop,user}-{find,show} commands. Thanks. >From b16972503f0401b4e9d286d0cd5110b14b2d8dc1 Mon Sep 17 00:00:00 2001 From: Pavel Reichl Date: Mon, 5 Oct 2015 10:02:05 -0400 Subject: [PATCH] intg: Add test for user and group local overrides

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-22 Thread Pavel Reichl
Hello, please see updated patch. Thanks! >From c51b34be1adbfa5266b4ac03abb48d74b6e5c348 Mon Sep 17 00:00:00 2001 From: Pavel Reichl Date: Mon, 5 Oct 2015 10:02:05 -0400 Subject: [PATCH] intg: Add test for user and group local overrides Introduce a new integration test for

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-21 Thread Nikolai Kondrashov
On 10/20/2015 05:15 PM, Pavel Reichl wrote: On 10/20/2015 03:12 PM, Nikolai Kondrashov wrote: Hi Pavel, On 10/13/2015 12:22 AM, Pavel Reichl wrote: Please see updated patchset. Thank you for working on this. Please see my comments below. First of all, we need the test in Makefile.am, so

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-21 Thread Nikolai Kondrashov
On 10/20/2015 08:36 PM, Pavel Reichl wrote: Nick I went ahead and started to reworking the patch in the direction you proposed. Could you check that this wip patch is what you had in mind? Thanks! Sure! Overall, I think the general scheme of separating tests you're doing is great! See below

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-21 Thread Pavel Reichl
On 10/21/2015 11:31 AM, Nikolai Kondrashov wrote: On 10/20/2015 05:15 PM, Pavel Reichl wrote: On 10/20/2015 03:12 PM, Nikolai Kondrashov wrote: Hi Pavel, On 10/13/2015 12:22 AM, Pavel Reichl wrote: Please see updated patchset. Thank you for working on this. Please see my comments

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-20 Thread Pavel Reichl
On 10/20/2015 03:12 PM, Nikolai Kondrashov wrote: Hi Pavel, On 10/13/2015 12:22 AM, Pavel Reichl wrote: Please see updated patchset. Thank you for working on this. Please see my comments below. First of all, we need the test in Makefile.am, so it is included into distribution. I'm not

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-20 Thread Pavel Reichl
All of the above said, I would really prefer to see this test split into a few smaller ones, each invoking a (possibly shared) py.test fixture separately. Since py.test doesn't really support nested tests (argh!), we'll need to have them flat. E.g.: * simple override (both with and

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-20 Thread Nikolai Kondrashov
Hi Pavel, On 10/13/2015 12:22 AM, Pavel Reichl wrote: Please see updated patchset. Thank you for working on this. Please see my comments below. First of all, we need the test in Makefile.am, so it is included into distribution. I'm not sure if we really need this split into three patches,

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-12 Thread Pavel Reichl
On 10/07/2015 06:35 PM, Nikolai Kondrashov wrote: On 10/07/2015 07:10 PM, Pavel Reichl wrote: We can use either mine (after modifications) or yours. I suppose yours are closer to be acked so we should probably use yours. I'm not so sure, but alright :) All valid comments, but I will

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-07 Thread Lukas Slebodnik
On (07/10/15 14:25), Pavel Reichl wrote: >Hello, > >please see first version of patch set. Please consider this to be work in >progress. > >The test 'test_local_override_group' fails, there seem to be a bug in >sss_override. Pavel is working on the patch. He kindly provided me with early

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-07 Thread Pavel Reichl
On 10/07/2015 02:55 PM, Lukas Slebodnik wrote: On (07/10/15 14:25), Pavel Reichl wrote: Hello, please see first version of patch set. Please consider this to be work in progress. The test 'test_local_override_group' fails, there seem to be a bug in sss_override. Pavel is working on the

[SSSD] [PATCHSET] intg: sss_override

2015-10-07 Thread Pavel Reichl
Hello, please see first version of patch set. Please consider this to be work in progress. The test 'test_local_override_group' fails, there seem to be a bug in sss_override. Pavel is working on the patch. He kindly provided me with early version of patch and it fixed the test. Currently

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-07 Thread Nikolai Kondrashov
On 10/07/2015 03:25 PM, Pavel Reichl wrote: Hello, please see first version of patch set. Please consider this to be work in progress. The test 'test_local_override_group' fails, there seem to be a bug in sss_override. Pavel is working on the patch. He kindly provided me with early version

Re: [SSSD] [PATCHSET] intg: sss_override

2015-10-07 Thread Nikolai Kondrashov
On 10/07/2015 07:10 PM, Pavel Reichl wrote: We can use either mine (after modifications) or yours. I suppose yours are closer to be acked so we should probably use yours. I'm not so sure, but alright :) All valid comments, but I will ignore them for now as I suppose this patch will be