Hi list,
I was debug code very deeply and I found nitpick. There is simple patch attached.
Regards
On (18/07/16 07:48), Petr Cech wrote:
Hi list,
I was debug code very deeply and I found nitpick. There is simple patch attached.
Regards
-- Petr^4 Čech
From 15d35c7e0e2171c49746d3a87f9aa2aba07a7bbc Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Sun, 17 Jul 2016 11:36:18 +0200 Subject: [PATCH] PROVIDERS: Default values in debug
It could be usefull to distinguish between deafult and config values in debug messages.
src/providers/data_provider_opts.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/providers/data_provider_opts.c b/src/providers/data_provider_opts.c index 9db43fc4077d3b14f2358029c7d532e440930879..d375c779a75ac1103efd8766026ed6604c1ca0f9 100644 --- a/src/providers/data_provider_opts.c +++ b/src/providers/data_provider_opts.c @@ -219,9 +219,10 @@ static int dp_copy_options_ex(TALLOC_CTX *memctx, opts[i].opt_name); goto done; }
DEBUG(SSSDBG_TRACE_FUNC, "Option %s has%s value %s\n",
DEBUG(SSSDBG_TRACE_FUNC, "Option %s has%s %svalue %s\n", opts[i].opt_name, opts[i].val.cstring ? "" : " no",
copy_values ? "" : "default ",
What is a benefit of string "default ". How does it improve reading debug messages with very high debug level? Why should we care wheter it's default or not?
LS
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
Regards
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
LS
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
On (29/07/16 10:05), Petr Cech wrote:
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
Would you be so kind and could you explain why it is importatn from developers point of view?
The default values are described in man page and moreover developer can see default in header/implementation files.
And I still miss a benefit of seeing the default value in log. Neither for developer not for other users. Could you elaborate a little bit. Or could you give few examples when is such information useful?
LS
On 07/29/2016 10:17 AM, Lukas Slebodnik wrote:
On (29/07/16 10:05), Petr Cech wrote:
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
Would you be so kind and could you explain why it is importatn from developers point of view?
The default values are described in man page and moreover developer can see default in header/implementation files.
And I still miss a benefit of seeing the default value in log. Neither for developer not for other users. Could you elaborate a little bit. Or could you give few examples when is such information useful?
The patch don't show default values as such. It just tags the default values. I was in situation that I needed to know what values comes from config file and what values are from default 'automagic'. Yes I could study every option one by one in man page or in code. But I just added this tag. That's all. I don't have any other reason or something like that.
If you think that I use wrong way how to obtain information about defaults/non-defaults -- just say it.
On the other hand, I think it might be a little shortcut for less experienced to tag the default options in debug message.
I am sorry that's all that I have to this topic. And if you think that such improvement is needless, just nack it.
Regards
On (29/07/16 10:46), Petr Cech wrote:
On 07/29/2016 10:17 AM, Lukas Slebodnik wrote:
On (29/07/16 10:05), Petr Cech wrote:
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote:
What is a benefit of string "default ".
Benefit is that we provide more information.
How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
Would you be so kind and could you explain why it is importatn from developers point of view?
The default values are described in man page and moreover developer can see default in header/implementation files.
And I still miss a benefit of seeing the default value in log. Neither for developer not for other users. Could you elaborate a little bit. Or could you give few examples when is such information useful?
The patch don't show default values as such. It just tags the default values. I was in situation that I needed to know what values comes from config file and what values are from default 'automagic'. Yes I could study every option one by one in man page or in code. But I just added this tag. That's all. I don't have any other reason or something like that.
If you think that I use wrong way how to obtain information about defaults/non-defaults -- just say it.
On the other hand, I think it might be a little shortcut for less experienced to tag the default options in debug message.
I am sorry that's all that I have to this topic. And if you think that such improvement is needless, just nack it.
I'm sorry but I am still not persuaded about benefit of this patch. As I previously wrote the string "default" or "non-defaults". The value is already in log file and it's enought for debugging.
I'm sorry but I cannot give an ACK
LS
On 08/05/2016 03:58 PM, Lukas Slebodnik wrote:
On (29/07/16 10:46), Petr Cech wrote:
On 07/29/2016 10:17 AM, Lukas Slebodnik wrote:
On (29/07/16 10:05), Petr Cech wrote:
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote:
On 07/29/2016 09:23 AM, Lukas Slebodnik wrote: > What is a benefit of string "default ".
Benefit is that we provide more information.
> How does it improve reading debug messages with very high debug level?
This patch doesn't improve reading in such way.
> Why should we care wheter it's default or not?
I was in situation during debug that I needed to know if value was default or not.
It does not explain why we should care wheter it's default or not.
This debug messages have so high level that it will not visible in standard deployment. Is there reason why we would like not to say what is default?
The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
Would you be so kind and could you explain why it is importatn from developers point of view?
The default values are described in man page and moreover developer can see default in header/implementation files.
And I still miss a benefit of seeing the default value in log. Neither for developer not for other users. Could you elaborate a little bit. Or could you give few examples when is such information useful?
The patch don't show default values as such. It just tags the default values. I was in situation that I needed to know what values comes from config file and what values are from default 'automagic'. Yes I could study every option one by one in man page or in code. But I just added this tag. That's all. I don't have any other reason or something like that.
If you think that I use wrong way how to obtain information about defaults/non-defaults -- just say it.
On the other hand, I think it might be a little shortcut for less experienced to tag the default options in debug message.
I am sorry that's all that I have to this topic. And if you think that such improvement is needless, just nack it.
I'm sorry but I am still not persuaded about benefit of this patch. As I previously wrote the string "default" or "non-defaults". The value is already in log file and it's enought for debugging.
I'm sorry but I cannot give an ACK
LS
OK, Lukas, thank you for review.
On 08/08/2016 01:47 PM, Petr Cech wrote:
On 08/05/2016 03:58 PM, Lukas Slebodnik wrote:
On (29/07/16 10:46), Petr Cech wrote:
On 07/29/2016 10:17 AM, Lukas Slebodnik wrote:
On (29/07/16 10:05), Petr Cech wrote:
On 07/29/2016 09:52 AM, Lukas Slebodnik wrote:
On (29/07/16 09:41), Petr Cech wrote: > On 07/29/2016 09:23 AM, Lukas Slebodnik wrote: >> What is a benefit of string "default ". > > Benefit is that we provide more information. > >> How does it improve reading debug messages with very high debug >> level? > > This patch doesn't improve reading in such way. > >> Why should we care wheter it's default or not? > > I was in situation during debug that I needed to know if value > was default or > not. > It does not explain why we should care wheter it's default or not.
> This debug messages have so high level that it will not visible > in standard > deployment. Is there reason why we would like not to say what is > default? > The default values are described in man page and from sssd point of view it does not matter whether the value is default or or it was explicitly set to default value in configuration file or it has non-default value.
I'm sorry I asked first and I didn't get a sufficient answer. So one more time Why should we care whether it's default or not?
From SSSD point of view we don't care if value is default or not. It doesn't influence logic of sssd.
From developer point of view it might be relevant. And that's my motivation for sending it.
Would you be so kind and could you explain why it is importatn from developers point of view?
The default values are described in man page and moreover developer can see default in header/implementation files.
And I still miss a benefit of seeing the default value in log. Neither for developer not for other users. Could you elaborate a little bit. Or could you give few examples when is such information useful?
The patch don't show default values as such. It just tags the default values. I was in situation that I needed to know what values comes from config file and what values are from default 'automagic'. Yes I could study every option one by one in man page or in code. But I just added this tag. That's all. I don't have any other reason or something like that.
If you think that I use wrong way how to obtain information about defaults/non-defaults -- just say it.
On the other hand, I think it might be a little shortcut for less experienced to tag the default options in debug message.
I am sorry that's all that I have to this topic. And if you think that such improvement is needless, just nack it.
I'm sorry but I am still not persuaded about benefit of this patch. As I previously wrote the string "default" or "non-defaults". The value is already in log file and it's enought for debugging.
I'm sorry but I cannot give an ACK
LS
OK, Lukas, thank you for review.
Even though I don't have anything against this patch I do not see the need for this either. We already have sssd.conf available, so it is easy to check which options are set and which are not. But as you said, it doesn't influence sssd in any way so it doesn't matter.
Maybe it will help if you describe the situation where this information was relevant.
sssd-devel@lists.fedorahosted.org