arrangement 6: add deprecation warnings to conf.get_* #765

mlangsdorf · 2018-05-18T14:03:36Z

A lot of conf arguments have been deprecated and moved to reactor_config_map.
Replace _get_value with get_deprecated and print a warning message for
all get* functions in conf where the value is not in the accepted parameters
list for BuildRequestV2.

Signed-off-by: Mark Langsdorf mlangsdo@redhat.com

stickler-ci · 2018-05-18T14:03:44Z

osbs/conf.py

-        return self._get_value("odcs_insecure", self.conf_section, "odcs_insecure",
-                               default=False, is_bool_val=True)
+        return self._get_deprecated("odcs_insecure", self.conf_section, "odcs_insecure",
+                                     default=False, is_bool_val=True)


E127 continuation line over-indented for visual indent

stickler-ci · 2018-05-18T14:03:45Z

osbs/conf.py


    def get_odcs_openidc_secret(self):
-        return self._get_value("odcs_openidc_secret", self.conf_section, "odcs_openidc_secret")
+        return self._get_deprecated("odcs_openidc_secret", self.conf_section,
+                                     "odcs_openidc_secret")


E127 continuation line over-indented for visual indent

stickler-ci · 2018-05-18T14:03:45Z

osbs/conf.py

-        return self._get_value("pdc_insecure", self.conf_section, "pdc_insecure",
-                               default=False, is_bool_val=True)
+        return self._get_deprecated("pdc_insecure", self.conf_section, "pdc_insecure",
+                                     default=False, is_bool_val=True)


E127 continuation line over-indented for visual indent

lcarva · 2018-05-21T12:50:42Z

osbs/conf.py

@@ -102,6 +102,12 @@ def get_value_from_conf():
        else:
            return value

+    def _get_deprecated(self, args_key, conf_section, conf_key, default=None, is_bool_val=False):
+        logger.warning("user configuration %s in section %s is ignored in arrangement %s",
+                       args_key, conf_section, DEFAULT_ARRANGEMENT_VERSION)


What happens when the default is arrangement 7? Maybe mention that this is true for arrangement versions greater than or equal to REACTOR_CONFIG_ARRANGEMENT_VERSION?

lcarva · 2018-05-21T13:37:20Z

osbs/conf.py


    def get_username(self):
-        return self._get_value("username", self.conf_section, "username")
+        return self._get_deprecated("username", self.conf_section, "username")


This is used for authenticating against OpenShift instance. It's still used by osbs-client when creating builds, if username/password auth is to be used.

lcarva · 2018-05-21T13:37:24Z

osbs/conf.py


    def get_password(self):
-        return self._get_value("password", self.conf_section, "password")
+        return self._get_deprecated("password", self.conf_section, "password")


This is password for authenticating against OpenShift instance. It's still used by osbs-client when creating builds, if username/password auth is to be used.

lcarva · 2018-05-21T13:40:06Z

osbs/conf.py


    def get_client_cert(self):
-        return self._get_value("client_cert", self.conf_section, "client_cert")
+        return self._get_deprecated("client_cert", self.conf_section, "client_cert")


This is cert for authenticating against OpenShift instance. It's still used by osbs-client when creating builds, if cert auth is to be used.

lcarva · 2018-05-21T13:40:15Z

osbs/conf.py


    def get_client_key(self):
-        return self._get_value("client_key", self.conf_section, "client_key")
+        return self._get_deprecated("client_key", self.conf_section, "client_key")


This is key for authenticating against OpenShift instance. It's still used by osbs-client when creating builds, if cert auth is to be used.

lcarva · 2018-05-21T13:50:53Z

osbs/conf.py


    def get_build_imagestream(self):
-        return self._get_value("build_imagestream", self.conf_section, "build_imagestream")
+        return self._get_deprecated("build_imagestream", self.conf_section, "build_imagestream")


This is used for creating builds, it must be in osbs-client.

lcarva · 2018-05-21T13:51:00Z

osbs/conf.py


    def get_build_from(self):
-        return self._get_value("build_from", self.conf_section, "build_from")
+        return self._get_deprecated("build_from", self.conf_section, "build_from")


This is used for creating builds, it must be in osbs-client.

lcarva · 2018-05-21T13:51:54Z

osbs/conf.py


    def get_client_config_secret(self):
-        return self._get_value("client_config_secret", self.conf_section,
-                               "client_config_secret")
+        return self._get_deprecated("client_config_secret", self.conf_section,


~~We're still using this in arrangement 6.~~ But it's now configured in reactor config map, so let's deprecate it here.

lcarva · 2018-05-21T13:52:35Z

osbs/conf.py

@@ -529,25 +535,23 @@ def get_arrangement_version(self):
            raise OsbsValidationException("Invalid arrangement_version: %s" % value)

    def get_can_orchestrate(self):
-        return self._get_value("can_orchestrate", self.conf_section, "can_orchestrate",
-                               default=False, is_bool_val=True)
+        return self._get_deprecated("can_orchestrate", self.conf_section, "can_orchestrate",


We need to know this before we can create the Build object in OpenShift. It shouldn't be deprecated.

lcarva · 2018-05-21T13:54:49Z

osbs/conf.py

        if value:
            return [x.strip() for x in value.split(',')]
        else:
            return []

    def get_equal_labels(self):
        equal_labels = []
-        equal_labels_str = self._get_value("equal_labels", self.conf_section,
-                                           "equal_labels")
+        equal_labels_str = self._get_deprecated("equal_labels", self.conf_section, "equal_labels")
        if equal_labels_str:
            # must be in correct format
            # e.g. `name1:name2:name3, release1:release2, version1:version2`


I can't comment any further, but shouldn't get_platform_descriptors also be deprecated now?

I think I got platform_descriptors and node_selectors confused. Fixed along with all the others.

lcarva · 2018-05-21T18:09:16Z

Not sure why the centos 6 build failed. Restarted it to make sure it's not one of those random failures.

lcarva · 2018-05-21T20:37:46Z

Ok, some progress in figuring out what's happening in CentOS 6.
It turns out to be a know pip issue (pypa/setuptools#937).
The workaround? Re-install pip via get-pip.py script 😨

twaugh · 2018-05-22T15:04:25Z

See #766 for CentOS 6 test fix.

A lot of conf arguments have been deprecated and moved to reactor_config_map. Replace _get_value with _get_deprecated and print a warning message for all get_* functions in conf where the value is not in the accepted parameters list for BuildRequestV2. Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>

mlangsdorf · 2018-05-22T19:39:04Z

added to the release notes under "Improvements":

added warning messages when a deprecated config variable is used

stickler-ci reviewed May 18, 2018

View reviewed changes

mlangsdorf force-pushed the osbs5056 branch from ec3221c to fc65e1c Compare May 18, 2018 15:48

mlangsdorf requested review from sosiouxme, lcarva, twaugh and rcerven May 21, 2018 11:40

lcarva reviewed May 21, 2018

View reviewed changes

mlangsdorf force-pushed the osbs5056 branch from fc65e1c to 9efaaee Compare May 21, 2018 15:19

lcarva approved these changes May 21, 2018

View reviewed changes

mlangsdorf force-pushed the osbs5056 branch from 9efaaee to f22b5fb Compare May 21, 2018 15:50

rcerven approved these changes May 22, 2018

View reviewed changes

mlangsdorf force-pushed the osbs5056 branch from f22b5fb to e81db2f Compare May 22, 2018 17:58

mlangsdorf merged commit 6044d0d into containerbuildsystem:master May 22, 2018

mlangsdorf deleted the osbs5056 branch June 27, 2018 13:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

arrangement 6: add deprecation warnings to conf.get_* #765

arrangement 6: add deprecation warnings to conf.get_* #765

mlangsdorf commented May 18, 2018

stickler-ci May 18, 2018

stickler-ci May 18, 2018

stickler-ci May 18, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

lcarva May 21, 2018

mlangsdorf May 21, 2018

lcarva commented May 21, 2018

lcarva commented May 21, 2018

twaugh commented May 22, 2018

mlangsdorf commented May 22, 2018

arrangement 6: add deprecation warnings to conf.get_* #765

arrangement 6: add deprecation warnings to conf.get_* #765

Conversation

mlangsdorf commented May 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcarva commented May 21, 2018

lcarva commented May 21, 2018

twaugh commented May 22, 2018

mlangsdorf commented May 22, 2018