[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization lslebodn commented: """ master: * cbee11e912bb391ba254b0bac8c1159c1f634533 sssd-1-14: * ec1829de7cd529c2c68b4bdb9b6d43ac6bb545d3 LS """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-256679524 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization lslebodn commented: """ ACK """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-256677906 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization lslebodn commented: """ We(mzidek, lslebodn) tried some feature for github. "Allow edits from maintainers." > When you are creating a new pull request, you'll see a checkbox > > labelled "Allow edits from maintainers". This is enabled by default. > > > > With this in place, anyone with commit access to the repository that > > is the target of the pull request will also be able to push changes > > to the branch of the repository that is the origin of the pull > > request. > But information about this changes does not appear in the conversation itself. Anyway we renamed the flag into `SSS_TOOL_FLAG_SKIP_CMD_INIT` """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-256040056 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ Is it really that confusing even with the comments added? Well, I immediately return from the function when the flag is set, because the function currently only initializes the confdb and the domain context. Of course it is possible that the function will do also something else in the future, but is not doing so now, so I do not see reason to put more code to the if-statement. I can change name of the function or flag (or any name) if you believe it will help readability, but please provide suggestions. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-254781287 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48
Title: #48: sssctl: Flags for commadn initialization
lslebodn commented:
"""
On (17/10/16 09:33), Jakub Hrozek wrote:
>I don't know how to unstuck this PR except providing some ideas
> * SSS_TOOL_FLAG_NOCONF
> * SSS_TOOL_FLAG_STATIC
> * SSS_TOOL_FLAG_CONFPARSE_FALSE
>
Let me explain how I read code of the function tool_cmd_init
+static int tool_cmd_init(struct sss_tool_ctx *tool_ctx,
+ struct sss_route_cmd *command)
+{
+int ret;
+
+if (command->flags & SSS_TOOL_FLAG_NOCONF) {
+/* This tool does not need to connect to confdb or
+ * initialize the domain contexts. Nothing to do. */
+return EOK;
The function tool_cmd_init has "init" in a name so I assume
it want to do some initialisation. The first action in the function
is checking "some flag". If the flag is enabled then the execution
is immediatelly finished with success.
Based on the name of flag SSS_TOOL_FLAG_NOCONF. It seems that
there should be some configuration done in this function which
should be skipped. However, this function does not do any configuration.
The previous name was SSS_TOOL_CMD_FLAG_NO_CONFDB which looks like
just confd shoudl not be initialized. But tool_cmd_init initialize two things
and can initialize more things in future. And it isn't important
that initialization of domain depends on confdb because it can change
in future.
If you would like to use the name of option SSS_TOOL_CMD_FLAG_NO_CONFDB
The code should look:
if ((command->flags & SSS_TOOL_FLAG_NOCONF) == 0) {
//initialisation of confdb
}
//further initialisation.
But your code loks like
if (command->flags & SSS_TOOL_FLAG_WITH_CONFUSING_NAME) {
//immediatelly finish
}
//initialisation of confdb
//initialisation of domains
http://martinfowler.com/bliki/TwoHardThings.html
"""
See the full comment at
https://github.com/SSSD/sssd/pull/48#issuecomment-254718848
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ I used the jhrozek's proposed *_NOCONF name. I also added comment to make it clear that it will not initialize both confdb and domain context. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-254483980 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization jhrozek commented: """ I don't know how to unstuck this PR except providing some ideas * SSS_TOOL_FLAG_NOCONF * SSS_TOOL_FLAG_STATIC * SSS_TOOL_FLAG_CONFPARSE_FALSE """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-254260235 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization jhrozek commented: """ On Fri, Oct 14, 2016 at 08:03:20AM -0700, mzidek-rh wrote: > I see the comment did not get forwarded to the devel list, so pasting again: > > > Sorry, I am out of ideas here. What name do you propose? Or will it be enough > if I just add a comment to the flag that it will not initialize the domain > context? > The review comments added to the github review tool do not make it to the list. Unfortunately github doesn't offer an API to catch the review comments.. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253831521 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ I see the comment did not get forwarded to the devel list, so pasting again: Sorry, I am out of ideas here. What name do you propose? Or will it be enough if I just add a comment to the flag that it will not initialize the domain context? """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253826861 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ Ok, new patch uploaded. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253774056 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization lslebodn commented: """ IMHO, it would be nicer if you add new macro SSS_TOOL_COMMAND_FLAGS (or different name) rather then changing usage of SSS_TOOL_COMMAND and SSS_TOOL_COMMAND_NOMSG on all places. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253767686 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ updated """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253487693 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ Updated patch according to Fabiano's comments. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253467053 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization
URL: https://github.com/SSSD/sssd/pull/48 Title: #48: sssctl: Flags for commadn initialization mzidek-rh commented: """ Hi, connecting to confdb makes some sssctl commands less useful (config-check and in the future, config-show). This adds flag to skip confdb init for sssctl commands. """ See the full comment at https://github.com/SSSD/sssd/pull/48#issuecomment-253186900 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
