Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
Ok, I added patch that removes the option from sssd.conf in integration tests.
Michal
On (27/07/15 15:58), Michal Židek wrote:
On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
Ok, I added patch that removes the option from sssd.conf in integration tests.
Michal
-- Senior Principal Intern
From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH 1/2] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 36df6ae..d2d7ebf 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -38,6 +38,7 @@
- @{
*/
+#define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" #define SSSD_MIN_ID 1 diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 93a1a1b..360e3a8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
ret = sss_ini_check_config_obj(init_data); if (ret != EOK) {
/* No known version. Assumed to be version 1 */
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- }
- version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
} else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
} else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
} else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
}
}
/* Set up a transaction to replace the configuration */
diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py b/src/config/SSSDConfig/sssd_upgrade_config.py index 282d6c4..767d06d 100644 --- a/src/config/SSSDConfig/sssd_upgrade_config.py +++ b/src/config/SSSDConfig/sssd_upgrade_config.py @@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf): def get_version(self): ver = self.get_option_index('sssd', 'config_file_version')[1] if not ver:
return 1
# config_file_version not found -> default to version 2
return 2 try: return int(ver['value']) except ValueError:
-- 2.1.0
From 52fb274e36c104a4b7d2df932d81a7ae07c008fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 27 Jul 2015 14:22:08 +0200 Subject: [PATCH 2/2] tests: Remove config_file_version from sssd.conf in CI
Default config_file_version was changed to 2 so the explicitly setting the value is no longer necessary. This change also serves as a test to see that the new default works.
Ticket: https://fedorahosted.org/sssd/ticket/2688
I prefer if so small change in test is part of "feature patch" So you do not forget to backport it.
LS
On 07/27/2015 04:05 PM, Lukas Slebodnik wrote:
On (27/07/15 15:58), Michal Židek wrote:
On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
Ok, I added patch that removes the option from sssd.conf in integration tests.
Michal
-- Senior Principal Intern
From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH 1/2] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 36df6ae..d2d7ebf 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -38,6 +38,7 @@
- @{
*/
+#define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" #define SSSD_MIN_ID 1 diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 93a1a1b..360e3a8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
ret = sss_ini_check_config_obj(init_data); if (ret != EOK) {
/* No known version. Assumed to be version 1 */
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- }
- version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
} else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
} else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
} else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
}
}
/* Set up a transaction to replace the configuration */
diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py b/src/config/SSSDConfig/sssd_upgrade_config.py index 282d6c4..767d06d 100644 --- a/src/config/SSSDConfig/sssd_upgrade_config.py +++ b/src/config/SSSDConfig/sssd_upgrade_config.py @@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf): def get_version(self): ver = self.get_option_index('sssd', 'config_file_version')[1] if not ver:
return 1
# config_file_version not found -> default to version 2
return 2 try: return int(ver['value']) except ValueError:
-- 2.1.0
From 52fb274e36c104a4b7d2df932d81a7ae07c008fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 27 Jul 2015 14:22:08 +0200 Subject: [PATCH 2/2] tests: Remove config_file_version from sssd.conf in CI
Default config_file_version was changed to 2 so the explicitly setting the value is no longer necessary. This change also serves as a test to see that the new default works.
Ticket: https://fedorahosted.org/sssd/ticket/2688
I prefer if so small change in test is part of "feature patch" So you do not forget to backport it.
Squashed.
Michal
On (27/07/15 16:12), Michal Židek wrote:
On 07/27/2015 04:05 PM, Lukas Slebodnik wrote:
On (27/07/15 15:58), Michal Židek wrote:
On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
Ok, I added patch that removes the option from sssd.conf in integration tests.
Michal
-- Senior Principal Intern
From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH 1/2] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 36df6ae..d2d7ebf 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -38,6 +38,7 @@
- @{
*/
+#define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" #define SSSD_MIN_ID 1 diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 93a1a1b..360e3a8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
ret = sss_ini_check_config_obj(init_data); if (ret != EOK) {
/* No known version. Assumed to be version 1 */
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- }
- version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
- } else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
^^ I do not prefer nested "if"s. If you decided to do it in this way then you shoudl have proper indentation.
goto done;
} else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
^^^ and also here.
LS
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
On (27/07/15 16:12), Michal Židek wrote:
On 07/27/2015 04:05 PM, Lukas Slebodnik wrote:
On (27/07/15 15:58), Michal Židek wrote:
On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
On (07/07/15 19:45), Michal Židek wrote:
Hi!
See the attached patch.
Ticket: https://fedorahosted.org/sssd/ticket/2688
CI passed: http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
Thanks, Michal
-- Senior Principal Intern
From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
I think you can also remove config_file_version from sssd.conf in integration test. Or at least comment it out. So it will be proven that your patch works.
LS
Ok, I added patch that removes the option from sssd.conf in integration tests.
Michal
-- Senior Principal Intern
From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH 1/2] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
src/confdb/confdb.h | 1 + src/confdb/confdb_setup.c | 48 ++++++++++++++-------------- src/config/SSSDConfig/sssd_upgrade_config.py | 3 +- 3 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h index 36df6ae..d2d7ebf 100644 --- a/src/confdb/confdb.h +++ b/src/confdb/confdb.h @@ -38,6 +38,7 @@
- @{
*/
+#define CONFDB_DEFAULT_CFG_FILE_VER 2 #define CONFDB_FILE "config.ldb" #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf" #define SSSD_MIN_ID 1 diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c index 93a1a1b..360e3a8 100644 --- a/src/confdb/confdb_setup.c +++ b/src/confdb/confdb_setup.c @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
ret = sss_ini_check_config_obj(init_data); if (ret != EOK) {
/* No known version. Assumed to be version 1 */
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- }
- version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
- if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
goto done;
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
- } else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
^^
I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
goto done;
} else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
^^^ and also here.
LS
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
- } else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
^^
I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
Please also remove config_file_version from test_memory_cache.py
LS
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
- } else if (version < CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file is an old version. "
"Please run configuration upgrade script.\n");
ret = EINVAL;
goto done;
- } else if (version > CONFDB_VERSION_INT) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version is newer than confdb\n");
ret = EINVAL;
goto done;
/* No known version. Use default. */
DEBUG(SSSDBG_CONF_SETTINGS,
"Value of config_file_version option not found. "
"Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
- } else {
version = sss_ini_get_int_config_value(init_data,
CONFDB_DEFAULT_CFG_FILE_VER,
-1, &ret);
if (ret != EOK) {
DEBUG(SSSDBG_FATAL_FAILURE,
"Config file version could not be determined\n");
^^
I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary little inconveniences.
Please also remove config_file_version from test_memory_cache.py
We do not necessary need to have all tests without the option to test if the default works. But I removed it from test_memory_cache.py anyway.
Michal
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
>- } else if (version < CONFDB_VERSION_INT) { >- DEBUG(SSSDBG_FATAL_FAILURE, >- "Config file is an old version. " >- "Please run configuration upgrade script.\n"); >- ret = EINVAL; >- goto done; >- } else if (version > CONFDB_VERSION_INT) { >- DEBUG(SSSDBG_FATAL_FAILURE, >- "Config file version is newer than confdb\n"); >- ret = EINVAL; >- goto done; >+ /* No known version. Use default. */ >+ DEBUG(SSSDBG_CONF_SETTINGS, >+ "Value of config_file_version option not found. " >+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >+ } else { >+ version = sss_ini_get_int_config_value(init_data, >+ CONFDB_DEFAULT_CFG_FILE_VER, >+ -1, &ret); >+ if (ret != EOK) { >+ DEBUG(SSSDBG_FATAL_FAILURE, >+ "Config file version could not be determined\n");
^^
I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
>>- } else if (version < CONFDB_VERSION_INT) { >>- DEBUG(SSSDBG_FATAL_FAILURE, >>- "Config file is an old version. " >>- "Please run configuration upgrade script.\n"); >>- ret = EINVAL; >>- goto done; >>- } else if (version > CONFDB_VERSION_INT) { >>- DEBUG(SSSDBG_FATAL_FAILURE, >>- "Config file version is newer than confdb\n"); >>- ret = EINVAL; >>- goto done; >>+ /* No known version. Use default. */ >>+ DEBUG(SSSDBG_CONF_SETTINGS, >>+ "Value of config_file_version option not found. " >>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>+ } else { >>+ version = sss_ini_get_int_config_value(init_data, >>+ CONFDB_DEFAULT_CFG_FILE_VER, >>+ -1, &ret); >>+ if (ret != EOK) { >>+ DEBUG(SSSDBG_FATAL_FAILURE, >>+ "Config file version could not be determined\n");
^^
I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
>>>- } else if (version < CONFDB_VERSION_INT) { >>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>- "Config file is an old version. " >>>- "Please run configuration upgrade script.\n"); >>>- ret = EINVAL; >>>- goto done; >>>- } else if (version > CONFDB_VERSION_INT) { >>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>- "Config file version is newer than confdb\n"); >>>- ret = EINVAL; >>>- goto done; >>>+ /* No known version. Use default. */ >>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>+ "Value of config_file_version option not found. " >>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>+ } else { >>>+ version = sss_ini_get_int_config_value(init_data, >>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>+ -1, &ret); >>>+ if (ret != EOK) { >>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>+ "Config file version could not be determined\n"); ^^ I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
you shoudl have proper indentation.
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote:
On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>- } else if (version < CONFDB_VERSION_INT) { >>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>- "Config file is an old version. " >>>>- "Please run configuration upgrade script.\n"); >>>>- ret = EINVAL; >>>>- goto done; >>>>- } else if (version > CONFDB_VERSION_INT) { >>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>- "Config file version is newer than confdb\n"); >>>>- ret = EINVAL; >>>>- goto done; >>>>+ /* No known version. Use default. */ >>>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>>+ "Value of config_file_version option not found. " >>>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>+ } else { >>>>+ version = sss_ini_get_int_config_value(init_data, >>>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>>+ -1, &ret); >>>>+ if (ret != EOK) { >>>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>>+ "Config file version could not be determined\n"); > ^^ >I do not prefer nested "if"s. If you decided to do it in this way then
Me neither, but sss_ini_get_int_config_value() has to be skipped conditionally. It is just call to the function plus error checking that is nested. I think it is not too bad in this case.
>you shoudl have proper indentation. >
Fixed in the new version.
It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
On (05/08/15 13:41), Michal Židek wrote: > On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>> - } else if (version < CONFDB_VERSION_INT) { >>>>> - DEBUG(SSSDBG_FATAL_FAILURE, >>>>> - "Config file is an old version. " >>>>> - "Please run configuration upgrade script.\n"); >>>>> - ret = EINVAL; >>>>> - goto done; >>>>> - } else if (version > CONFDB_VERSION_INT) { >>>>> - DEBUG(SSSDBG_FATAL_FAILURE, >>>>> - "Config file version is newer than confdb\n"); >>>>> - ret = EINVAL; >>>>> - goto done; >>>>> + /* No known version. Use default. */ >>>>> + DEBUG(SSSDBG_CONF_SETTINGS, >>>>> + "Value of config_file_version option not found. " >>>>> + "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>> + } else { >>>>> + version = sss_ini_get_int_config_value(init_data, >>>>> + CONFDB_DEFAULT_CFG_FILE_VER, >>>>> + -1, &ret); >>>>> + if (ret != EOK) { >>>>> + DEBUG(SSSDBG_FATAL_FAILURE, >>>>> + "Config file version could not be determined\n"); >> ^^ >> I do not prefer nested "if"s. If you decided to do it in this way then > > Me neither, but sss_ini_get_int_config_value() has to be > skipped conditionally. It is just call to the function > plus error checking that is nested. I think it is not > too bad in this case. > >> you shoudl have proper indentation. >> > > Fixed in the new version. > It works because integration tests passed. http://sssd-ci.duckdns.org/logs/job/20/68/summary.html But ...
I tested new version with ipa-client-install and "config_file_version = 2" is still added to sssd.conf even though it is a default value.
ipa-client-install uses our python API (python-sssdconfig) and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
On (01/09/15 12:55), Michal Židek wrote:
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote:
On 08/06/2015 01:39 PM, Lukas Slebodnik wrote: >On (05/08/15 13:41), Michal Židek wrote: >>On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>>>- } else if (version < CONFDB_VERSION_INT) { >>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>- "Config file is an old version. " >>>>>>- "Please run configuration upgrade script.\n"); >>>>>>- ret = EINVAL; >>>>>>- goto done; >>>>>>- } else if (version > CONFDB_VERSION_INT) { >>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>- "Config file version is newer than confdb\n"); >>>>>>- ret = EINVAL; >>>>>>- goto done; >>>>>>+ /* No known version. Use default. */ >>>>>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>>>>+ "Value of config_file_version option not found. " >>>>>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>>>+ } else { >>>>>>+ version = sss_ini_get_int_config_value(init_data, >>>>>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>>>>+ -1, &ret); >>>>>>+ if (ret != EOK) { >>>>>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>+ "Config file version could not be determined\n"); >>> ^^ >>>I do not prefer nested "if"s. If you decided to do it in this way then >> >>Me neither, but sss_ini_get_int_config_value() has to be >>skipped conditionally. It is just call to the function >>plus error checking that is nested. I think it is not >>too bad in this case. >> >>>you shoudl have proper indentation. >>> >> >>Fixed in the new version. >> >It works because integration tests passed. >http://sssd-ci.duckdns.org/logs/job/20/68/summary.html >But ... > >I tested new version with ipa-client-install >and "config_file_version = 2" is still added to sssd.conf >even though it is a default value. > >ipa-client-install uses our python API (python-sssdconfig) >and it does not try to add this option itself.
I often change version of SSSD with git checkout sssd<version>. If I generate the config with realmd or ipa-client-install
realmd do not use python module SSSDConfig.
with latest version then the config_version_file would need to be added manually (and I am pretty sure it would be after I looked into logs to see why sssd is not starting). I know this will probably only hit testers/developers, but I would prefer not to add unnecessary
just developers and developers useually join machine to AD or IPA just once.
little inconveniences.
I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
* integration tests passed * ipa-client generated config without config_file_version and sssd worked.
ACK
LS
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
On (01/09/15 12:55), Michal Židek wrote:
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
On (10/08/15 17:18), Michal Židek wrote: >On 08/06/2015 01:39 PM, Lukas Slebodnik wrote: >>On (05/08/15 13:41), Michal Židek wrote: >>>On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>>>>- } else if (version < CONFDB_VERSION_INT) { >>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>- "Config file is an old version. " >>>>>>>- "Please run configuration upgrade script.\n"); >>>>>>>- ret = EINVAL; >>>>>>>- goto done; >>>>>>>- } else if (version > CONFDB_VERSION_INT) { >>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>- "Config file version is newer than confdb\n"); >>>>>>>- ret = EINVAL; >>>>>>>- goto done; >>>>>>>+ /* No known version. Use default. */ >>>>>>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>>>>>+ "Value of config_file_version option not found. " >>>>>>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>>>>+ } else { >>>>>>>+ version = sss_ini_get_int_config_value(init_data, >>>>>>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>>>>>+ -1, &ret); >>>>>>>+ if (ret != EOK) { >>>>>>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>+ "Config file version could not be determined\n"); >>>> ^^ >>>>I do not prefer nested "if"s. If you decided to do it in this way then >>> >>>Me neither, but sss_ini_get_int_config_value() has to be >>>skipped conditionally. It is just call to the function >>>plus error checking that is nested. I think it is not >>>too bad in this case. >>> >>>>you shoudl have proper indentation. >>>> >>> >>>Fixed in the new version. >>> >>It works because integration tests passed. >>http://sssd-ci.duckdns.org/logs/job/20/68/summary.html >>But ... >> >>I tested new version with ipa-client-install >>and "config_file_version = 2" is still added to sssd.conf >>even though it is a default value. >> >>ipa-client-install uses our python API (python-sssdconfig) >>and it does not try to add this option itself. > >I often change version of SSSD with git checkout sssd<version>. >If I generate the config with realmd or ipa-client-install realmd do not use python module SSSDConfig.
>with latest version then the config_version_file >would need to be added manually (and I am pretty >sure it would be after I looked into logs to see why sssd >is not starting). I know this will probably only hit >testers/developers, but I would prefer not to add unnecessary just developers and developers useually join machine to AD or IPA just once.
>little inconveniences. > I do not try old sssd version very often. Just in case of bisect. and ther is nice/simple workarount for "little inconvenience"
Just manually add "config_version_file = 2" to sssd.conf
The main point of this ticket is to simplify sssd.conf and our python sssdconfig API should do the same. Otherwise we do not need to do such change.
I still see "config_file_version = 2" in sssd.conf It was generated by python module SSSDConfig (via ipa-client-install)
LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
- integration tests passed
- ipa-client generated config without config_file_version and sssd worked.
ACK
LS
* master: 175613be0cfb0890174d12d941e634d833b63dd9
Michal, can you rebase the patch for sssd-1-12 ?
On (03/09/15 09:35), Jakub Hrozek wrote:
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
On (01/09/15 12:55), Michal Židek wrote:
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote:
On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote: >On (10/08/15 17:18), Michal Židek wrote: >>On 08/06/2015 01:39 PM, Lukas Slebodnik wrote: >>>On (05/08/15 13:41), Michal Židek wrote: >>>>On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>>>>>- } else if (version < CONFDB_VERSION_INT) { >>>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>- "Config file is an old version. " >>>>>>>>- "Please run configuration upgrade script.\n"); >>>>>>>>- ret = EINVAL; >>>>>>>>- goto done; >>>>>>>>- } else if (version > CONFDB_VERSION_INT) { >>>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>- "Config file version is newer than confdb\n"); >>>>>>>>- ret = EINVAL; >>>>>>>>- goto done; >>>>>>>>+ /* No known version. Use default. */ >>>>>>>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>>>>>>+ "Value of config_file_version option not found. " >>>>>>>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>>>>>+ } else { >>>>>>>>+ version = sss_ini_get_int_config_value(init_data, >>>>>>>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>>>>>>+ -1, &ret); >>>>>>>>+ if (ret != EOK) { >>>>>>>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>+ "Config file version could not be determined\n"); >>>>> ^^ >>>>>I do not prefer nested "if"s. If you decided to do it in this way then >>>> >>>>Me neither, but sss_ini_get_int_config_value() has to be >>>>skipped conditionally. It is just call to the function >>>>plus error checking that is nested. I think it is not >>>>too bad in this case. >>>> >>>>>you shoudl have proper indentation. >>>>> >>>> >>>>Fixed in the new version. >>>> >>>It works because integration tests passed. >>>http://sssd-ci.duckdns.org/logs/job/20/68/summary.html >>>But ... >>> >>>I tested new version with ipa-client-install >>>and "config_file_version = 2" is still added to sssd.conf >>>even though it is a default value. >>> >>>ipa-client-install uses our python API (python-sssdconfig) >>>and it does not try to add this option itself. >> >>I often change version of SSSD with git checkout sssd<version>. >>If I generate the config with realmd or ipa-client-install >realmd do not use python module SSSDConfig. > >>with latest version then the config_version_file >>would need to be added manually (and I am pretty >>sure it would be after I looked into logs to see why sssd >>is not starting). I know this will probably only hit >>testers/developers, but I would prefer not to add unnecessary >just developers and developers useually join machine >to AD or IPA just once. > >>little inconveniences. >> >I do not try old sssd version very often. Just in case of bisect. >and ther is nice/simple workarount for "little inconvenience" > >Just manually add "config_version_file = 2" to sssd.conf > >The main point of this ticket is to simplify sssd.conf >and our python sssdconfig API should do the same. >Otherwise we do not need to do such change. > >I still see "config_file_version = 2" in sssd.conf >It was generated by python module SSSDConfig (via ipa-client-install) > >LS
This thread has stalled, let's try to restart it.
What versions are normally still being worked on by developers? I suspect it's master and sssd-1-12. What about pushing the patch to sssd-1-12 as well?
I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
- integration tests passed
- ipa-client generated config without config_file_version and sssd worked.
ACK
LS
- master: 175613be0cfb0890174d12d941e634d833b63dd9
Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12
LS
On 09/03/2015 09:58 AM, Lukas Slebodnik wrote:
On (03/09/15 09:35), Jakub Hrozek wrote:
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
On (01/09/15 12:55), Michal Židek wrote:
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
On (01/09/15 10:51), Jakub Hrozek wrote: > On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote: >> On (10/08/15 17:18), Michal Židek wrote: >>> On 08/06/2015 01:39 PM, Lukas Slebodnik wrote: >>>> On (05/08/15 13:41), Michal Židek wrote: >>>>> On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>>>>>> - } else if (version < CONFDB_VERSION_INT) { >>>>>>>>> - DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>> - "Config file is an old version. " >>>>>>>>> - "Please run configuration upgrade script.\n"); >>>>>>>>> - ret = EINVAL; >>>>>>>>> - goto done; >>>>>>>>> - } else if (version > CONFDB_VERSION_INT) { >>>>>>>>> - DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>> - "Config file version is newer than confdb\n"); >>>>>>>>> - ret = EINVAL; >>>>>>>>> - goto done; >>>>>>>>> + /* No known version. Use default. */ >>>>>>>>> + DEBUG(SSSDBG_CONF_SETTINGS, >>>>>>>>> + "Value of config_file_version option not found. " >>>>>>>>> + "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>>>>>> + } else { >>>>>>>>> + version = sss_ini_get_int_config_value(init_data, >>>>>>>>> + CONFDB_DEFAULT_CFG_FILE_VER, >>>>>>>>> + -1, &ret); >>>>>>>>> + if (ret != EOK) { >>>>>>>>> + DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>> + "Config file version could not be determined\n"); >>>>>> ^^ >>>>>> I do not prefer nested "if"s. If you decided to do it in this way then >>>>> >>>>> Me neither, but sss_ini_get_int_config_value() has to be >>>>> skipped conditionally. It is just call to the function >>>>> plus error checking that is nested. I think it is not >>>>> too bad in this case. >>>>> >>>>>> you shoudl have proper indentation. >>>>>> >>>>> >>>>> Fixed in the new version. >>>>> >>>> It works because integration tests passed. >>>> http://sssd-ci.duckdns.org/logs/job/20/68/summary.html >>>> But ... >>>> >>>> I tested new version with ipa-client-install >>>> and "config_file_version = 2" is still added to sssd.conf >>>> even though it is a default value. >>>> >>>> ipa-client-install uses our python API (python-sssdconfig) >>>> and it does not try to add this option itself. >>> >>> I often change version of SSSD with git checkout sssd<version>. >>> If I generate the config with realmd or ipa-client-install >> realmd do not use python module SSSDConfig. >> >>> with latest version then the config_version_file >>> would need to be added manually (and I am pretty >>> sure it would be after I looked into logs to see why sssd >>> is not starting). I know this will probably only hit >>> testers/developers, but I would prefer not to add unnecessary >> just developers and developers useually join machine >> to AD or IPA just once. >> >>> little inconveniences. >>> >> I do not try old sssd version very often. Just in case of bisect. >> and ther is nice/simple workarount for "little inconvenience" >> >> Just manually add "config_version_file = 2" to sssd.conf >> >> The main point of this ticket is to simplify sssd.conf >> and our python sssdconfig API should do the same. >> Otherwise we do not need to do such change. >> >> I still see "config_file_version = 2" in sssd.conf >> It was generated by python module SSSDConfig (via ipa-client-install) >> >> LS > > This thread has stalled, let's try to restart it. > > What versions are normally still being worked on by developers? I > suspect it's master and sssd-1-12. What about pushing the patch to > sssd-1-12 as well? I'm fine with sssd-1-12. The python sssdconfig need to be updated.
LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
- integration tests passed
- ipa-client generated config without config_file_version and sssd worked.
ACK
LS
- master: 175613be0cfb0890174d12d941e634d833b63dd9
Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12
LS
Indeed it was the tests.
Patch for 1.12 is attached.
Michal
On Thu, Sep 03, 2015 at 02:29:16PM +0200, Michal Židek wrote:
On 09/03/2015 09:58 AM, Lukas Slebodnik wrote:
On (03/09/15 09:35), Jakub Hrozek wrote:
On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
On (01/09/15 12:55), Michal Židek wrote:
On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote: >On (01/09/15 10:51), Jakub Hrozek wrote: >>On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote: >>>On (10/08/15 17:18), Michal Židek wrote: >>>>On 08/06/2015 01:39 PM, Lukas Slebodnik wrote: >>>>>On (05/08/15 13:41), Michal Židek wrote: >>>>>>On 07/30/2015 06:08 PM, Lukas Slebodnik wrote: >>>>>>>>>>- } else if (version < CONFDB_VERSION_INT) { >>>>>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>>>- "Config file is an old version. " >>>>>>>>>>- "Please run configuration upgrade script.\n"); >>>>>>>>>>- ret = EINVAL; >>>>>>>>>>- goto done; >>>>>>>>>>- } else if (version > CONFDB_VERSION_INT) { >>>>>>>>>>- DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>>>- "Config file version is newer than confdb\n"); >>>>>>>>>>- ret = EINVAL; >>>>>>>>>>- goto done; >>>>>>>>>>+ /* No known version. Use default. */ >>>>>>>>>>+ DEBUG(SSSDBG_CONF_SETTINGS, >>>>>>>>>>+ "Value of config_file_version option not found. " >>>>>>>>>>+ "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER); >>>>>>>>>>+ } else { >>>>>>>>>>+ version = sss_ini_get_int_config_value(init_data, >>>>>>>>>>+ CONFDB_DEFAULT_CFG_FILE_VER, >>>>>>>>>>+ -1, &ret); >>>>>>>>>>+ if (ret != EOK) { >>>>>>>>>>+ DEBUG(SSSDBG_FATAL_FAILURE, >>>>>>>>>>+ "Config file version could not be determined\n"); >>>>>>> ^^ >>>>>>>I do not prefer nested "if"s. If you decided to do it in this way then >>>>>> >>>>>>Me neither, but sss_ini_get_int_config_value() has to be >>>>>>skipped conditionally. It is just call to the function >>>>>>plus error checking that is nested. I think it is not >>>>>>too bad in this case. >>>>>> >>>>>>>you shoudl have proper indentation. >>>>>>> >>>>>> >>>>>>Fixed in the new version. >>>>>> >>>>>It works because integration tests passed. >>>>>http://sssd-ci.duckdns.org/logs/job/20/68/summary.html >>>>>But ... >>>>> >>>>>I tested new version with ipa-client-install >>>>>and "config_file_version = 2" is still added to sssd.conf >>>>>even though it is a default value. >>>>> >>>>>ipa-client-install uses our python API (python-sssdconfig) >>>>>and it does not try to add this option itself. >>>> >>>>I often change version of SSSD with git checkout sssd<version>. >>>>If I generate the config with realmd or ipa-client-install >>>realmd do not use python module SSSDConfig. >>> >>>>with latest version then the config_version_file >>>>would need to be added manually (and I am pretty >>>>sure it would be after I looked into logs to see why sssd >>>>is not starting). I know this will probably only hit >>>>testers/developers, but I would prefer not to add unnecessary >>>just developers and developers useually join machine >>>to AD or IPA just once. >>> >>>>little inconveniences. >>>> >>>I do not try old sssd version very often. Just in case of bisect. >>>and ther is nice/simple workarount for "little inconvenience" >>> >>>Just manually add "config_version_file = 2" to sssd.conf >>> >>>The main point of this ticket is to simplify sssd.conf >>>and our python sssdconfig API should do the same. >>>Otherwise we do not need to do such change. >>> >>>I still see "config_file_version = 2" in sssd.conf >>>It was generated by python module SSSDConfig (via ipa-client-install) >>> >>>LS >> >>This thread has stalled, let's try to restart it. >> >>What versions are normally still being worked on by developers? I >>suspect it's master and sssd-1-12. What about pushing the patch to >>sssd-1-12 as well? >I'm fine with sssd-1-12. >The python sssdconfig need to be updated. > >LS
Michal, if you agree, please update the patches so that we can push them in time for sssd-1.13.1
New patch attached. I tested ipa-client-install and the config_file_version is no longer added.
Michal
From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Tue, 7 Jul 2015 15:15:32 +0200 Subject: [PATCH] CONFDB: Assume config file version 2 if missing
Default to config file version 2 if the version is not specified explicitly.
Ticket: https://fedorahosted.org/sssd/ticket/2688
- integration tests passed
- ipa-client generated config without config_file_version and sssd worked.
ACK
LS
- master: 175613be0cfb0890174d12d941e634d833b63dd9
Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12
LS
Indeed it was the tests.
Patch for 1.12 is attached.
Michal
CI had some issues, but I don't think those are related to your patch: http://sssd-ci.duckdns.org/logs/job/26/52/summary.html
ACK for sssd-1-12
On Mon, Sep 14, 2015 at 11:10:53AM +0200, Jakub Hrozek wrote:
CI had some issues, but I don't think those are related to your patch: http://sssd-ci.duckdns.org/logs/job/26/52/summary.html
ACK for sssd-1-12
* sssd-1-12: c35eb4aa64a67d34d343d608be40d60b61fb7d11
sssd-devel@lists.fedorahosted.org