[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured

[amitkumar50]

  URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured

amitkumar50 commented:
"""
@jhrozek Done changes. Thanks
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/515#issuecomment-372545291
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#515][synchronized] sssctl: Showing help even when sssd not configured

[amitkumar50]

   URL: https://github.com/SSSD/sssd/pull/515
Author: amitkumar50
 Title: #515: sssctl: Showing help even when sssd not configured
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/515/head:pr515
git checkout pr515
From c5cbcdbebb753cec5ed0c918b5d00a1ec97d3129 Mon Sep 17 00:00:00 2001
From: amitkuma 
Date: Thu, 15 Feb 2018 18:21:10 +0530
Subject: [PATCH] sssctl: Showing help even when sssd not configured

On a clean and unconfigured system, it's not possible
to use --help.
1) dnf install sssd-tools
2) sssctl cache-remove --help
Shows:
[confdb_get_domains] (0x0010): No domains configured, fatal error!

Solution: Donot check for confdb initialization when sssctl 3rd
command line argument passed is '--help'.

Please note when we run 'sssctl --help' on unconfigured system
confdb check is not done and proper o/p is seen.

Resolves: https://pagure.io/SSSD/sssd/issue/3634
---
 src/tools/common/sss_tools.c | 36 ++--
 src/tools/common/sss_tools.h |  5 +++--
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c
index e491a1286..aee6bfcae 100644
--- a/src/tools/common/sss_tools.c
+++ b/src/tools/common/sss_tools.c
@@ -53,16 +53,20 @@ static struct poptOption *sss_tool_common_opts_table(void)
 }
 
 static void sss_tool_common_opts(struct sss_tool_ctx *tool_ctx,
- int *argc, const char **argv)
+ int *argc, const char **argv,
+ bool *_help)
 {
 poptContext pc;
 int debug = SSSDBG_DEFAULT;
 int orig_argc = *argc;
+int help = 0;
 int opt;
 
 struct poptOption options[] = {
 {"debug", '\0', POPT_ARG_INT | POPT_ARGFLAG_STRIP, ,
 0, _("The debug level to run with"), NULL },
+{"help", '?', POPT_ARG_VAL | POPT_ARGFLAG_DOC_HIDDEN, ,
+1, NULL, NULL },
 POPT_TABLEEND
 };
 
@@ -74,6 +78,7 @@ static void sss_tool_common_opts(struct sss_tool_ctx *tool_ctx,
 /* Strip common options from arguments. We will discard_const here,
  * since it is not worth the trouble to convert it back and forth. */
 *argc = poptStrippedArgv(pc, orig_argc, discard_const_p(char *, argv));
+*_help = help;
 
 DEBUG_CLI_INIT(debug);
 
@@ -168,7 +173,8 @@ static errno_t sss_tool_domains_init(TALLOC_CTX *mem_ctx,
 
 errno_t sss_tool_init(TALLOC_CTX *mem_ctx,
   int *argc, const char **argv,
-  struct sss_tool_ctx **_tool_ctx)
+  struct sss_tool_ctx **_tool_ctx,
+  bool *_help)
 {
 struct sss_tool_ctx *tool_ctx;
 
@@ -178,8 +184,7 @@ errno_t sss_tool_init(TALLOC_CTX *mem_ctx,
 return ENOMEM;
 }
 
-sss_tool_common_opts(tool_ctx, argc, argv);
-
+sss_tool_common_opts(tool_ctx, argc, argv, _help);
 *_tool_ctx = tool_ctx;
 
 return EOK;
@@ -296,7 +301,7 @@ static int tool_cmd_init(struct sss_tool_ctx *tool_ctx,
 errno_t sss_tool_route(int argc, const char **argv,
struct sss_tool_ctx *tool_ctx,
struct sss_route_cmd *commands,
-   void *pvt)
+   void *pvt, bool help)
 {
 struct sss_cmdline cmdline;
 const char *cmd;
@@ -333,13 +338,15 @@ errno_t sss_tool_route(int argc, const char **argv,
 return tool_ctx->init_err;
 }
 
-ret = tool_cmd_init(tool_ctx, [i]);
-if (ret != EOK) {
-DEBUG(SSSDBG_FATAL_FAILURE,
-  "Command initialization failed [%d] %s\n",
-  ret, sss_strerror(ret));
-return ret;
-}
+	if (!help) {
+ret = tool_cmd_init(tool_ctx, [i]);
+if (ret != EOK) {
+DEBUG(SSSDBG_FATAL_FAILURE,
+  "Command initialization failed [%d] %s\n",
+  ret, sss_strerror(ret));
+return ret;
+}
+	}
 
 return commands[i].fn(, tool_ctx, pvt);
 }
@@ -494,6 +501,7 @@ int sss_tool_main(int argc, const char **argv,
 struct sss_tool_ctx *tool_ctx;
 uid_t uid;
 errno_t ret;
+bool _help = false;
 
 uid = getuid();
 if (uid != 0) {
@@ -502,7 +510,7 @@ int sss_tool_main(int argc, const char **argv,
 return EXIT_FAILURE;
 }
 
-ret = sss_tool_init(NULL, , argv, _ctx);
+ret = sss_tool_init(NULL, , argv, _ctx, &_help);
 if (ret == ERR_SYSDB_VERSION_TOO_OLD) {
 tool_ctx->init_err = ret;
 } else if (ret != EOK) {
@@ -510,7 +518,7 @@ int sss_tool_main(int argc, const char **argv,
 return EXIT_FAILURE;
 }
 
-ret = sss_tool_route(argc, argv, tool_ctx, commands, pvt);
+ret = sss_tool_route(argc, argv, tool_ctx, commands, pvt, _help);
 

[SSSD] [sssd PR#511][comment] Do not shutdown KCM/Secrets responders when activities are happening ...

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/511
Title: #511: Do not shutdown KCM/Secrets responders when activities are 
happening ...

fidencio commented:
"""
> why do the first two patches fix code that the subsequent patches refactor? 
> Is it to make it clear where the issue is? Or did you consider backporting 
> the first two patches to some older branches?

It was to make clear that the issues has been fixed in the first two patches. 
The rest of the series is "just" refactoring the code. IMO it's just easier for 
the "future fidencio" to bisect the some issue and understand why things where 
done in this way in the past.

So, answering your second question ... No, when I wrote the patches I didn't 
think of backporting them to any specific branch ... but had in mind that 
distros could choose to backport just those 2 patches instead of the whole 
refactor.

> I see you added a new dependency python-psutils but only is_running() is 
> used. Have you considered implementing a simple version of is_running 
> internally? I think just checking for the same PID and the same executable 
> and the same running time would be quite OK.

python-psutils was there for free (on all supported OSes in our CI) and that's 
the reason I've decided to use that.

IMO the effort of maintaining our on version of "is_running" (our anything else 
already provided by a package that is there from el6 to latest fedora) is not 
worth it, but if that's your preference ... I can write up my own and use it.

> The tests you added are nice and it's really great that the coverage is on 
> par with the code, but the tests are also slow. 90 and 60 seconds 
> respectively is quite a lot. I wonder if you could send a mail to sssd-devel 
> about this in case anyone would be irritated by slow tests. I think it 
> doesn't matter how long Jenkins is chewing the tests, but if I was hacking on 
> sssd-secrets, I wouldn't like to wait for over two minutes for tests to 
> complete the make intgcheck-run.

I'll send an email to the sssd-devel about this.
Another option would be to allow responders to run for less than 60 seconds 
(the minimum default time) ... let's say change it to 10 and then those tests 
would be way faster.

Would those changes be welcome? (I'll wait for your answer before dropping an 
email to sssd-devel).
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/511#issuecomment-372477710
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#531][comment] Add the needed machinery to have automated builds for our COPR repos

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/531
Title: #531: Add the needed machinery to have automated builds for our COPR 
repos

fidencio commented:
"""
> I'm fine with this plan for master, but do you think it's wise with our level 
> of testing to build packages after every commit?

Maybe I should be more conservative (and here I'm quite opened to be convinced 
to do so), but if someone decides to use sssd-1-13 or sssd-master **copr** they 
pretty much know what they signed for.

Those repos are **not** supposed to be delivered to customers, at all, but to 
help people help us with early test of a release (and here, again, they'd know 
what they signed up for).

Considering we don't have this for every commit, what would be your idea? What 
kind of content would be okay enough to trigger a update in a non supported 
copr repo? IMO ... if we don't do this for every commit (but do it monthly, 
let's say), I'd prefer to stick to do smaller releases, way more often and then 
make the copr repos something that's not even needed.

> We also try to check the greentea tests at least before releases to make sure 
> no regressions are introduced and sometimes this takes quite a bit of time 
> between the test breaks and the fix is introduced.

Yep, but here we're talking about releases. A copr repo is not an official 
release and should not be treated as so. Again, I do believe that people using 
it know what they're signing up for (and I may be wrong here :-))

> So I would vote to only use this mechanism for the master branch. If you 
> think there are users of the other branches for every commit, then I would 
> prefer to have a separate branch where we build only the releases and a 
> separate git-tracking branch that would be built automatically.

I don't see the point on not having sssd-1-13 releases for instance. Correct me 
if I'm mistaken, but I don't think this branch is currently tested apart from 
integration/unit tests when something new is pushed there ... by providing the 
copr repo someone would at least be able to give it a try (and it's totally for 
free for us).

Although, I'm not in the project for time enough to properly understand your 
concerns.
What are the problems you see that may hit us in the future in case we have the 
copr repos updated on every commit on those branches ... considering that 
releases will still happen on those and the releases are the "stable" version?

> btw now I wonder if the only-releases-go-here branch could also be automated 
> in the sense that a build would only happen in the branch when a new tag is 
> added..

I'm not sure how easy it would be, but I can spend some time investigating it.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/531#issuecomment-372472945
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#511][comment] Do not shutdown KCM/Secrets responders when activities are happening ...

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/511
Title: #511: Do not shutdown KCM/Secrets responders when activities are 
happening ...

jhrozek commented:
"""
Actually, I have some more questions about the code:
 - why do the first two patches fix code that the subsequent patches refactor? 
Is it to make it clear where the issue is? Or did you consider backporting the 
first two patches to some older branches?
 - I see you added a new dependency python-psutils but only is_running() is 
used. Have you considered implementing a simple version of is_running 
internally? I think just checking for the same PID and the same executable and 
the same running time would be quite OK.
 - The tests you added are nice and it's really great that the coverage is on 
par with the code, but the tests are also slow. 90 and 60 seconds respectively 
is quite a lot. I wonder if you could send a mail to sssd-devel about this in 
case anyone would be irritated by slow tests. I think it doesn't matter how 
long Jenkins is chewing the tests, but if I was hacking on sssd-secrets, I 
wouldn't like to wait for over two minutes for tests to complete the `make 
intgcheck-run`.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/511#issuecomment-372472897
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#519][comment] DEBUG: Print simple access provider allow and deny lists

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/519
Title: #519: DEBUG: Print simple access provider allow and deny lists

jhrozek commented:
"""
ACK, CI http://vm-031.${ABC}/logs/job/86/34/summary.html
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/519#issuecomment-372449692
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#525][+Pushed] TESTS: simple CA to generate certificates for test

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/525
Title: #525: TESTS: simple CA to generate certificates for test

Label: +Pushed
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#519][+Accepted] DEBUG: Print simple access provider allow and deny lists

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/519
Title: #519: DEBUG: Print simple access provider allow and deny lists

Label: +Accepted
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#525][comment] TESTS: simple CA to generate certificates for test

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/525
Title: #525: TESTS: simple CA to generate certificates for test

jhrozek commented:
"""
Unsuprisingly the PR doesn't apply to older branches. I agree it would be nice 
to backport it to sssd-1-13, because it's the long-term branch and the 
workaround was valid only until May-2018 IIRC. @sumit-bose would you prefer to 
do the backport yourself or should someone else do it?

I would personally not care about sssd-1-14 unless it's very little work 
compared to the sssd-1-13 branch. IMO we should just release what we have and 
announce that users should migrate to other supported branches.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/525#issuecomment-372447719
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#528][-Accepted] NSS: Adjust netgroup setnetgrent cache lifetime if midpoint refresh is used

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/528
Title: #528: NSS: Adjust netgroup setnetgrent cache lifetime if midpoint 
refresh is used

Label: -Accepted
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#528][+Changes requested] NSS: Adjust netgroup setnetgrent cache lifetime if midpoint refresh is used

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/528
Title: #528: NSS: Adjust netgroup setnetgrent cache lifetime if midpoint 
refresh is used

Label: +Changes requested
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#528][comment] NSS: Adjust netgroup setnetgrent cache lifetime if midpoint refresh is used

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/528
Title: #528: NSS: Adjust netgroup setnetgrent cache lifetime if midpoint 
refresh is used

jhrozek commented:
"""
I'm sorry for the delay.

I think in this case the minimal timeout makes sense. Considering the NSS API, 
which has a separate `setnetgrent` call to just start working with a netgroup 
and a separate `getnetgrent` call to get netgroup data, I think the short 
timeout might cause too many rebuilds of the internal representation.

So I'm going to add the short timeout back. It's really just an odd edge case, 
anyway.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/528#issuecomment-372445608
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#529][comment] SPEC: Move secrets responder to the package sssd-kcm

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/529
Title: #529: SPEC: Move secrets responder to the package sssd-kcm

jhrozek commented:
"""
I understand the point about minimal dependencies of sssd-common, but I wonder 
if it was better to create a separate subpackage for the secrets binary. This 
is for two reasons, a) the proposed packaging is not intuitive, b) we are 
actually thinking about not talking to the secrets REST API from KCM to 
secrets, but writing to the secrets database directly. And then bundling the 
two responders together wouldn't make sense.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/529#issuecomment-372444317
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#531][comment] Add the needed machinery to have automated builds for our COPR repos

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/531
Title: #531: Add the needed machinery to have automated builds for our COPR 
repos

jhrozek commented:
"""
I'm fine with this plan for master, but do you think it's wise with our level 
of testing to build packages after every commit? We also try to check the 
greentea tests at least before releases to make sure no regressions are 
introduced and sometimes this takes quite a bit of time between the test breaks 
and the fix is introduced.

So I would vote to only use this mechanism for the master branch. If you think 
there are users of the other branches for every commit, then I would prefer to 
have a separate branch where we build only the releases and a separate 
git-tracking branch that would be built automatically.

btw now I wonder if the only-releases-go-here branch could also be automated in 
the sense that a build would only happen in the branch when a new tag is added..
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/531#issuecomment-372442416
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [SSSD-users] Re: Announcing SSSD 1.16.1

[Joakim Tjernlund]

On Sun, 2018-03-11 at 21:38 +0100, Jakub Hrozek wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe.
> 
> 
> > On 9 Mar 2018, at 14:45, Joakim Tjernlund  
> > wrote:
> > 
> > On Fri, 2018-03-09 at 13:28 +0100, Jakub Hrozek wrote:
> > > CAUTION: This email originated from outside of the organization. Do not 
> > > click links or open attachments unless you recognize the sender and know 
> > > the content is safe.
> > > 
> > > 
> > > SSSD 1.16.1
> > > ===
> > > 
> > > The SSSD team is proud to announce the release of version 1.16.1 of the
> > > System Security Services Daemon.
> > > 
> > > The tarball can be downloaded from https://releases.pagure.org/SSSD/sssd/
> > > 
> > > RPM packages will be made available for Fedora shortly.
> > > 
> > > Feedback
> > > 
> > > Please provide comments, bugs and other feedback
> > > via the sssd-devel or sssd-users mailing lists:
> > >   https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >   https://lists.fedorahosted.org/mailman/listinfo/sssd-users
> > > 
> > 
> > Did a quick test here and it seems like enumerate = true is
> > broken. Is it just me or .. ?
> 
> I don’t know about any bugs around enumeration in 1.16.1. Maybe you found an 
> issue, but it’s hard to say without more context.

OK, thanks.
I am a bit pressed for time but I did install 1.16.1 on another machine as well 
and now I see
a pattern:
I cleared the sss/db and rebooted, logged in and tested again with good old 
finger command
and it failed, I waited 5-10 mins and finger still failed. Went on lunch and
when I got back finger worked!

It seems that enumerate can take a very long time?
sssd.conf(minor edits):

[sssd]
config_file_version = 2
domains = xxx.com
services = nss, pam
#debug_level = 0x0fff

[nss]
fallback_homedir = /home/%u
default_shell = /bin/bash
#debug_level = 0x0fff
enum_cache_timeout = 3600
entry_negative_timeout = 300

[pam]
#debug_level = 0x0fff

[domain/xxx.com]
#debug_level = 0x

timeout = 30
ad_maximum_machine_account_password_age = 0

ignore_group_members = false
ldap_id_mapping = false
cache_credentials = true
enumerate = false
ldap_enumeration_refresh_timeout = 1800
entry_cache_timeout = 3600
refresh_expired_interval = 2700

id_provider = ad
auth_provider = ad
access_provider = permit
chpass_provider = ad

dyndns_update = true
dyndns_refresh_interval = 600
dyndns_update_ptr = true
dyndns_ttl = 3600
case_sensitive = false

ldap_referrals = false
ldap_sasl_mech = GSSAPI
ldap_schema = rfc2307bis

ldap_access_order = expire
ldap_account_expire_policy = ad
ldap_force_upper_case_realm = true

krb5_realm = .COM
krb5_canonicalize = true
krb5_store_password_if_offline = true
krb5_use_kdcinfo = False
krb5_renewable_lifetime = 7d
krb5_lifetime = 24h
krb5_renew_interval = 4h

 Jocke
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][closed] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[CendioOssman]

   URL: https://github.com/SSSD/sssd/pull/530
Author: CendioOssman
 Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/530/head:pr530
git checkout pr530
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

mzidek-rh commented:
"""
Actually, I do not have permissions to close this PR :D 

So, someone who does have it, please close this PR :)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372318447
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][+Rejected] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

Label: +Rejected
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

mzidek-rh commented:
"""
@simo5 `domain/_defaults_` sounds good to me as well :) , I will updated the 
issue and I am closing this PR.

Thanks everyone for your input.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372317234
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

mzidek-rh commented:
"""
Ok, I just pushed to last iteration to CI. If it is green I will ACK :)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372315576
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[simo5]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
No strong opinion beside bikeshedding on the name: ```domain/_defaults_``` :-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372307376
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[simo5]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
No strong opinion beside bikeshedding on the name: domain/_defaults_ :-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372307376
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

fidencio commented:
"""
Okay, let me go for Sumit's suggestion. ;-) I've updated the patch.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372306534
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][synchronized] TESTS: Fix E501 pep8 issues on test_netgroup.py

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/532
Author: fidencio
 Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/532/head:pr532
git checkout pr532
From 501cfcf585922957120f94d00cb6fdcbf88d9959 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 12 Mar 2018 08:54:54 +0100
Subject: [PATCH] TESTS: Fix E501 pep8 issues on test_netgroup.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

E501: line too long (longer than 79 characters)

The issue was inserted in commit 0f8add07b8, which has been pushed just
before the pep8 patches got merged.

The whole file was changed in order to adapt to the changes proposed to
this patch.

Signed-off-by: Fabiano Fidêncio 
---
 src/tests/intg/test_netgroup.py | 135 
 1 file changed, 68 insertions(+), 67 deletions(-)

diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py
index 06a1cfafd..d0e2c6a76 100644
--- a/src/tests/intg/test_netgroup.py
+++ b/src/tests/intg/test_netgroup.py
@@ -30,7 +30,8 @@
 import ds_openldap
 import ldap_ent
 from util import unindent
-import sssd_netgroup
+from sssd_nss import NssReturnCode
+from sssd_netgroup import get_sssd_netgroups
 
 LDAP_BASE_DN = "dc=example,dc=com"
 
@@ -210,8 +211,8 @@ def test_add_empty_netgroup(add_empty_netgroup):
 Adding empty netgroup.
 """
 
-res, _, netgroups = sssd_netgroup.get_sssd_netgroups("empty_netgroup")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgroups = get_sssd_netgroups("empty_netgroup")
+assert res == NssReturnCode.SUCCESS
 assert netgroups == []
 
 
@@ -244,29 +245,29 @@ def test_add_tripled_netgroup(add_tripled_netgroup):
 Adding netgroup with triplet.
 """
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup")
+assert res == NssReturnCode.SUCCESS
 assert netgrps == [("host", "user", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("adv_tripled_netgroup")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("adv_tripled_netgroup")
+assert res == NssReturnCode.SUCCESS
 assert sorted(netgrps) == sorted([("host1", "user1", "domain1"),
   ("host2", "user2", "domain2")])
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_domain")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_domain")
+assert res == NssReturnCode.SUCCESS
 assert netgrps == [("host", "user", "")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_user")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_user")
+assert res == NssReturnCode.SUCCESS
 assert netgrps == [("host", "", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_host")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_host")
+assert res == NssReturnCode.SUCCESS
 assert netgrps == [("", "user", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_none")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_none")
+assert res == NssReturnCode.SUCCESS
 assert netgrps == [("", "", "")]
 
 
@@ -308,43 +309,43 @@ def test_add_mixed_netgroup(add_mixed_netgroup):
 Adding many netgroups of different type.
 """
 
-res, _, netgroups = sssd_netgroup.get_sssd_netgroups("mixed_netgroup1")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgroups = get_sssd_netgroups("mixed_netgroup1")
+assert res == NssReturnCode.SUCCESS
 assert netgroups == []
 
-res, _, netgroups = sssd_netgroup.get_sssd_netgroups("mixed_netgroup2")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgroups = get_sssd_netgroups("mixed_netgroup2")
+assert res == NssReturnCode.SUCCESS
 assert netgroups == []
 
-res, _, netgroups = sssd_netgroup.get_sssd_netgroups("mixed_netgroup3")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgroups = get_sssd_netgroups("mixed_netgroup3")
+assert res == NssReturnCode.SUCCESS
 assert netgroups == [("host1", "user1", "domain1")]
 
-res, _, netgroups = sssd_netgroup.get_sssd_netgroups("mixed_netgroup4")
-assert res == sssd_netgroup.NssReturnCode.SUCCESS
+res, _, netgroups = get_sssd_netgroups("mixed_netgroup4")
+assert res == 

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

mzidek-rh commented:
"""
@simo5 : I understand why you want to set defaults in the [sssd] section and it 
makes sense to me. But I also think we may end up finding similar situations in 
the future (need defaults in [sssd] with the ability to override it in 
[domain/..] section) and I do not like it much, we basically double the same 
option.

Not just for this use case with gpo rule to pam service mappings, but for 
making the config file snippets more powerful, we could implement something 
like ` [domain/_GLOBAL_] ` that could set global defaults for all domains (and 
that would apply to all options available for domain section). I already opened 
this ticket:
https://pagure.io/SSSD/sssd/issue/3670

What do you think?


"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372304194
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

mzidek-rh commented:
"""
The last version does not work for me:
```
# PEP8_IGNORE="--ignore=E121,E123,E126,E226,E24,E704,W503"
# find ./src -path ./src/config -prune -o -name \*.py -exec pep8 $PEP8_IGNORE 
{} +
./src/tests/intg/test_netgroup.py:248:13: E127 continuation line over-indented 
for visual indent
./src/tests/intg/test_netgroup.py:253:13: E127 continuation line over-indented 
for visual indent
./src/tests/intg/test_netgroup.py:259:13: E127 continuation line over-indented 
for visual indent
./src/tests/intg/test_netgroup.py:264:13: E127 continuation line over-indented 
for visual indent
./src/tests/intg/test_netgroup.py:269:13: E127 continuation line over-indented 
for visual indent
./src/tests/intg/test_netgroup.py:274:13: E127 continuation line over-indented 
for visual indent
```

+1 for @sumit-bose 's suggestion with import (if there is some issue even with 
this, then we can push the original patch as fallback to unblock the CI)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372279616
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

sumit-bose commented:
"""
With 

diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py
index 3cf5dac..5c36dc2 100644
--- a/src/tests/intg/test_netgroup.py
+++ b/src/tests/intg/test_netgroup.py
@@ -30,7 +30,8 @@ import config
 import ds_openldap
 import ldap_ent
 from util import unindent
-import sssd_netgroup
+from sssd_nss import NssReturnCode
+from sssd_netgroup import get_sssd_netgroups

you can remove 'sssd_netgroup.' all over the place.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372278267
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

fidencio commented:
"""
@jhrozek, I like your suggestion more than mine.

I've updated the patch and pushed it to our internal CI. I'll get back here as 
soon as I have the results.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372276385
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[jhrozek]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

jhrozek commented:
"""
Feel free to push the patch as emergency to unblock the CI, but I admit I don't 
find the syntax very readable. What about:
```
--- a/src/tests/intg/test_netgroup.py
+++ b/src/tests/intg/test_netgroup.py
@@ -253,7 +253,9 @@ def test_add_tripled_netgroup(add_tripled_netgroup):
 assert sorted(netgrps) == sorted([("host1", "user1", "domain1"),
   ("host2", "user2", "domain2")])

-res, _, netgrps = 
sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_domain")
+res, _, netgrps = \
+sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_domain")
+
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("host", "user", "")]
```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372270891
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[mzidek-rh]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

mzidek-rh commented:
"""
I pushed the patch to CI.

This issue blocks CI, so I will ACK it when the results are green. I am not 
python expert either, but the fix LGTM. If someone proposes better solution in 
the meantime we can go with that, but unblocking the tests is more important 
now.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372265205
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:46 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 12:34, Pavel Březina  wrote:
>>
>> On 03/08/2018 12:22 PM, Jakub Hrozek wrote:
 On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:

 On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>
>> People,
>>
>> I've noticed that I'm getting a little bit lost with github and the
>> way SSSD has its tags organized there.
>>
>> As it may actually affect other people (and in case it does not, let's
>> just skip the following suggestion) ... I'd like to suggest the
>> following tags to the project:
>>
>> - Accepted: We already have it;
>>
>> - Rejected: We already have it.
>>
>> - Tests needed: This one can either replace the "Changes Requested"
>> (in case it's split in a few different tags) or be used together ...
>> but the idea is to identify that tests are missing from a PR without
>> going through the whole discussions happening there;
>
> What do you propose would be the action after tests needed? Should it be 
> a follow up PR, a ticket for the project, a ticket for downstream..?
>

 After the "Tests needed" tag is added the developer should either:
 - Write the tests upstream (considering that we have infra for that,
 which is not the case for all the PRs)
>>> Here I’m really worried that unless we have a ticket, this won’t happen. 
>>> Look at the “CI” milestone in pagure.. So I would say this case should 
>>> result in Changes requested, filing a ticket or asking downstream QE to 
>>> write a test.
>>
>> I would use this like this:
>>
>> a) You push a PR so it can be reviewed and you plan to provide tests later.
>>
>> b) You push a PR without tests and someone decides tests are mandatory for 
>> this PR.
>>
 - Provide a "link" of the related downstream tests that were
 broken/were added passing

>>> This makes sense, although I would argue this should already be default. 
>>> But if you don’t think so, we can try the tag and see how it goes.
 So, summing up, no ticket for the project, no ticket downstream ...
 just making clear that the PR is stalled because "Tests are needed".
 Does that make sense?

> My worry about not supplying tests along with PRs is that the tests will 
> never be supplied..at least not in upstream..

 I understand why you're worried and I agree with that. See the answer
 above and let me know if it fits your expectations.

>
>>
>> - Depends on (or something similar): This one can either replace the
>> "Changes Requested" (in case it's split in a few different tags) or be
>> used together ... but the idea is to identify that we depend on
>> somework that still has to be done (either another PR, ticket or
>> something else that has to be implemented). Mind that I'm not sure
>> whether we'd be able to simply add a field saying what the PR depends
>> on …
>
> I think this makes sense. At least for a casual observer it would be 
> clear that there is no work needed on this P >>>
>>
>> - Postponed/Deferred: We have something similar for 2.0, but would be
>> nice to have a way to clearly see in which release we're going to take
>> a look into a specific PR without having to dig in the discussions.
>> Here we could also have 1.16.1, 1.16.2, 2.0, …
>
> Tags are cheap, we can even have a postponed/$version. I guess even 
> depends/$PR might be OK as long as we only had a handful of dependecies.
>>
>> Patch dependencies are not that common so we can just set tag 
>> "depend/blocked" and write PR/ticket it depends on to the PR message as we 
>> do now.
>>
>>
>> - Reworked: Although just removing the "Changes Requested" label is
>> fine, maybe having a tag explicitly saying that something was Reworked
>> would be a clean way to differentiate between new PRs and PRs which
>> have been through a review already …
>
> I don’t know how this tag would be used, could you give me an example, 
> please?

 I usually have no idea (just by a quick look on github) whether a PR
 has been re-worked or it's a new PR that's never been reviewed.
 My impression is that having the "Reworked" tag would make simpler for
 people to jump in and do a follow-up review on what has been addressed
 in the first round(s) of review and then give their ACK instead of
 just leaving it for the reviewer. Of course, the same can be achieved
 without that tag ... so, it's just something that looks more
 "organized" to me.
>>> OK, if this is something that was hitting you, maybe the tag might make 
>>> sense. But, then do you volunteer to maintain these tags? Because since I 

[SSSD] Re: Github labels: Suggestion

[Fabiano Fidêncio]

On Thu, Mar 8, 2018 at 12:44 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 12:30, Fabiano Fidêncio  wrote:
>>
>> On Thu, Mar 8, 2018 at 12:22 PM, Jakub Hrozek  wrote:
>>>
>>>
 On 8 Mar 2018, at 12:13, Fabiano Fidêncio  wrote:

 On Thu, Mar 8, 2018 at 12:00 PM, Jakub Hrozek  wrote:
>
>
>> On 8 Mar 2018, at 10:33, Fabiano Fidêncio  wrote:
>>
>> People,
>>
>> I've noticed that I'm getting a little bit lost with github and the
>> way SSSD has its tags organized there.
>>
>> As it may actually affect other people (and in case it does not, let's
>> just skip the following suggestion) ... I'd like to suggest the
>> following tags to the project:
>>
>> - Accepted: We already have it;
>>
>> - Rejected: We already have it.
>>
>> - Tests needed: This one can either replace the "Changes Requested"
>> (in case it's split in a few different tags) or be used together ...
>> but the idea is to identify that tests are missing from a PR without
>> going through the whole discussions happening there;
>
> What do you propose would be the action after tests needed? Should it be 
> a follow up PR, a ticket for the project, a ticket for downstream..?
>

 After the "Tests needed" tag is added the developer should either:
 - Write the tests upstream (considering that we have infra for that,
 which is not the case for all the PRs)
>>>
>>> Here I’m really worried that unless we have a ticket, this won’t happen. 
>>> Look at the “CI” milestone in pagure.. So I would say this case should 
>>> result in Changes requested, filing a ticket or asking downstream QE to 
>>> write a test.
>>
>> Hmm. My original thought is that the PR wouldn't even be pushed
>> without the tests (as it happened already with a few PRs) ... not that
>> we'd push and leave the tag there.
>
> So it would mean “the code looks good and will be pushed as long as you sort 
> out tests one way or another” ?

I do think so. But this is something that must be agreed between us (the team).

>
>>
>>>
 - Provide a "link" of the related downstream tests that were
 broken/were added passing

>>>
>>> This makes sense, although I would argue this should already be default. 
>>> But if you don’t think so, we can try the tag and see how it goes.
>>
>> Hmm. Indeed. I guess we can postpone this at least for now and focus
>> on your "downstream tests passed" tag ... which would be a better
>> investment of time.
>> Agreed?
>
> Well, I don’t think the tests passed tag would come soon (as in, not this 
> week, not the next one). If you see a use-case for more tags, use them. But 
> as Pavel said, I would at least initially add a comment what do you want from 
> the other part of the PR until we find out how the process works best for us.
>
> I mean, I don’t want to impose my workflow on others. If you think it makes 
> sense, let’s try it. Worst thing, we stop using the tags and remove them..
>
>>
>>>
 So, summing up, no ticket for the project, no ticket downstream ...
 just making clear that the PR is stalled because "Tests are needed".
 Does that make sense?

> My worry about not supplying tests along with PRs is that the tests will 
> never be supplied..at least not in upstream..

 I understand why you're worried and I agree with that. See the answer
 above and let me know if it fits your expectations.

>
>>
>> - Depends on (or something similar): This one can either replace the
>> "Changes Requested" (in case it's split in a few different tags) or be
>> used together ... but the idea is to identify that we depend on
>> somework that still has to be done (either another PR, ticket or
>> something else that has to be implemented). Mind that I'm not sure
>> whether we'd be able to simply add a field saying what the PR depends
>> on …
>
> I think this makes sense. At least for a casual observer it would be 
> clear that there is no work needed on this PR.
>
>>
>> - Postponed/Deferred: We have something similar for 2.0, but would be
>> nice to have a way to clearly see in which release we're going to take
>> a look into a specific PR without having to dig in the discussions.
>> Here we could also have 1.16.1, 1.16.2, 2.0, …
>
> Tags are cheap, we can even have a postponed/$version. I guess even 
> depends/$PR might be OK as long as we only had a handful of dependecies.
>>
>> - Reworked: Although just removing the "Changes Requested" label is
>> fine, maybe having a tag explicitly saying that something was Reworked
>> would be a clean way to differentiate between new PRs and PRs which
>> have been through a review already …
>
> I don’t know how this tag would be used, could 

[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

fidencio commented:
"""
I'm adding the discussion here:
```
 simo: not going to extend the discussion in the pagure with a 
question that sounds quite stupid to me ... but why the option should be global?
 simo: I would easily buy if you tell me it should go under [pam] 
section as it's a list of PAM service names
 s/pagure/github PR/
 fidencio: because the association is not domain specific
 if a service drops a file it wants that association to be valid for any 
possible domain you may join with that machine
 it is a default mapping
 has nothing to do with which domain you specifically ended up joining
 simo: hmmm. I see your point. OTOH I can also think that as it's 
something AD specific ... it should go in the domain section (and I guess 
that's the reason it's there) (mind that I'm not advocating for the current 
behaviour ... just trying to see both sides)
 fidencio: a domain section is a specific instantiation
 we are talking about a global default behavior
 service X uses pam file Y
 that's true besides what specific instantiation you use
 although in some contrived cases you may want to override this behavior 
in a specific domain
 which is also why software should not drop a domain specific snippet as 
admins may have their own specific configs
 simo: okay, got your point
 simo: so, a good way to fix this is to move this option to a global 
section ...
 fidencio: we need this option *also* as a global, to set defaults
 then per domain for overrides/domain specific
```

And the discussion can be added to the ticket as soon as we specifically have a 
ticket for this and/or decide to use the documentation ticket for this (I'll 
leave this decision to @mzidek-rh).
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/530#issuecomment-372226106
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][comment] TESTS: Fix E501 pep8 issues on test_netgroup.py

[fidencio]

  URL: https://github.com/SSSD/sssd/pull/532
Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py

fidencio commented:
"""
Just a note here ... knowing my python knowledge I'd say there are a few other 
ways to better solve this issue. So, please, give me your suggestion and I'll 
change the patch accordingly. :-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/532#issuecomment-372224702
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#532][opened] TESTS: Fix E501 pep8 issues on test_netgroup.py

[fidencio]

   URL: https://github.com/SSSD/sssd/pull/532
Author: fidencio
 Title: #532: TESTS: Fix E501 pep8 issues on test_netgroup.py
Action: opened

PR body:
"""
E501: line too long (longer than 79 characters)

The issue was inserted in commit 0f8add07b8, which has been pushed just
before the pep8 patches got merged.

Signed-off-by: Fabiano Fidêncio 
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/532/head:pr532
git checkout pr532
From e0d102519bc32086a353a448d10e6e8e82a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 12 Mar 2018 08:54:54 +0100
Subject: [PATCH] TESTS: Fix E501 pep8 issues on test_netgroup.py
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

E501: line too long (longer than 79 characters)

The issue was inserted in commit 0f8add07b8, which has been pushed just
before the pep8 patches got merged.

Signed-off-by: Fabiano Fidêncio 
---
 src/tests/intg/test_netgroup.py | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/tests/intg/test_netgroup.py b/src/tests/intg/test_netgroup.py
index 06a1cfafd..8102bf617 100644
--- a/src/tests/intg/test_netgroup.py
+++ b/src/tests/intg/test_netgroup.py
@@ -244,28 +244,30 @@ def test_add_tripled_netgroup(add_tripled_netgroup):
 Adding netgroup with triplet.
 """
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup")
+get_sssd_netgroups = sssd_netgroup.get_sssd_netgroups
+
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("host", "user", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("adv_tripled_netgroup")
+res, _, netgrps = get_sssd_netgroups("adv_tripled_netgroup")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert sorted(netgrps) == sorted([("host1", "user1", "domain1"),
   ("host2", "user2", "domain2")])
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_domain")
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_domain")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("host", "user", "")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_user")
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_user")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("host", "", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_no_host")
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_no_host")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("", "user", "domain")]
 
-res, _, netgrps = sssd_netgroup.get_sssd_netgroups("tripled_netgroup_none")
+res, _, netgrps = get_sssd_netgroups("tripled_netgroup_none")
 assert res == sssd_netgroup.NssReturnCode.SUCCESS
 assert netgrps == [("", "", "")]
 
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org