From 54b257220f06b3eb215550bcc2f757d62d61d7dd Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 27 Mar 2023 11:10:30 +0100 Subject: [PATCH] Use custom warnings, not logging.warning We were using logging.warning() to warn the user about fields that had been removed in recent API versions or behavior that was now considered deprecated in SDK. This was the wrong API to use. We shouldn't have been logging, we have been using 'warnings'. From the Python docs [1]: Task you want to perform: Issue a warning regarding a particular runtime event warnings.warn() in library code if the issue is avoidable and the client application should be modified to eliminate the warning logging.warning() if there is nothing the client application can do about the situation, but the event should still be noted Based on this, introduce a new module, 'openstack.warnings', containing a number of custom 'DeprecationWarning' subclasses. 'DeprecationWarning' isn't show by default in most cases [2] but users can opt-in to showing them and do so selectively. For example, they may wish to ignore warnings about fields that have been removed in recent API versions while raising errors if they are relying on deprecated SDK behavior. [1] https://docs.python.org/3/howto/logging.html#when-to-use-logging [2] https://docs.python.org/3/library/exceptions.html#DeprecationWarning Change-Id: I3846e8fcffdb5de2afe64365952d90b5ecb0f74a Signed-off-by: Stephen Finucane --- doc/source/user/index.rst | 1 + doc/source/user/warnings.rst | 18 ++++++++++ openstack/resource.py | 35 +++++++++---------- openstack/warnings.py | 31 ++++++++++++++++ .../switch-to-warnings-333955d19afc99ca.yaml | 7 ++++ 5 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 doc/source/user/warnings.rst create mode 100644 openstack/warnings.py create mode 100644 releasenotes/notes/switch-to-warnings-333955d19afc99ca.yaml diff --git a/doc/source/user/index.rst b/doc/source/user/index.rst index 51a242047..a78322772 100644 --- a/doc/source/user/index.rst +++ b/doc/source/user/index.rst @@ -162,6 +162,7 @@ can be customized. resource service_description utils + warnings Presentations ------------- diff --git a/doc/source/user/warnings.rst b/doc/source/user/warnings.rst new file mode 100644 index 000000000..a0053a3a3 --- /dev/null +++ b/doc/source/user/warnings.rst @@ -0,0 +1,18 @@ +Warnings +======== + +openstacksdk uses the `warnings`__ infrastructure to warn users about +deprecated resources and resource fields, as well as deprecated behavior in +openstacksdk itself. Currently, these warnings are all derived from +``DeprecationWarning``. In Python, deprecation warnings are silenced by +default. You must turn them on using the ``-Wa`` Python command line option or +the ``PYTHONWARNINGS`` environment variable. If you are writing an application +that uses openstacksdk, you may wish to enable some of these warnings during +test runs to ensure you migrate away from deprecated behavior. + +Available warnings +------------------ + +.. automodule:: openstack.warnings + +.. __: https://docs.python.org/3/library/warnings.html diff --git a/openstack/resource.py b/openstack/resource.py index 3f02faddf..f5a205670 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -48,6 +48,7 @@ from openstack import _log from openstack import exceptions from openstack import format from openstack import utils +from openstack import warnings as os_warnings _SEEN_FORMAT = '{name}_seen' @@ -196,29 +197,27 @@ class _BaseComponent: return None # This warning are pretty intruisive. Every time attribute is accessed - # a warning is being thrown. In Neutron clients we have way too many - # places that still refer to tenant_id even they may also properly - # support project_id. For now we can silence tenant_id warnings and do - # this here rather then addining support for something similar to - # "suppress_deprecation_warning". + # a warning is being thrown. In neutron clients we have way too many + # places that still refer to tenant_id even though they may also + # properly support project_id. For now we silence tenant_id warnings. if self.name != "tenant_id": self.warn_if_deprecated_property(value) return _convert_type(value, self.type, self.list_type) def warn_if_deprecated_property(self, value): deprecated = object.__getattribute__(self, 'deprecated') - deprecate_reason = object.__getattribute__(self, 'deprecation_reason') + deprecation_reason = object.__getattribute__( + self, 'deprecation_reason', + ) - if value and deprecated and not self.already_warned_deprecation: - self.already_warned_deprecation = True - if not deprecate_reason: - LOG.warning( - "The option [%s] has been deprecated. " - "Please avoid using it.", + if value and deprecated: + warnings.warn( + "The field %r has been deprecated. %s" % ( self.name, - ) - else: - LOG.warning(deprecate_reason) + deprecation_reason or "Avoid usage." + ), + os_warnings.RemovedFieldWarning, + ) return value def __set__(self, instance, value): @@ -676,10 +675,10 @@ class Resource(dict): for attr, component in self._attributes_iterator(tuple([Body])): if component.name == name: warnings.warn( - 'Access to "%s[%s]" is deprecated. ' - 'Please access using "%s.%s" attribute.' + "Access to '%s[%s]' is deprecated. " + "Use '%s.%s' attribute instead" % (self.__class__, name, self.__class__, attr), - DeprecationWarning, + os_warnings.LegacyAPIWarning, ) return getattr(self, attr) if self._allow_unknown_attrs_in_body: diff --git a/openstack/warnings.py b/openstack/warnings.py new file mode 100644 index 000000000..b4be45549 --- /dev/null +++ b/openstack/warnings.py @@ -0,0 +1,31 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +class OpenStackDeprecationWarning(DeprecationWarning): + """Base class for warnings about deprecated features in openstacksdk.""" + + +class RemovedResourceWarning(OpenStackDeprecationWarning): + """Indicates that a resource has been removed in newer API versions and + should not be used. + """ + + +class RemovedFieldWarning(OpenStackDeprecationWarning): + """Indicates that a field has been removed in newer API versions and should + not be used. + """ + + +class LegacyAPIWarning(OpenStackDeprecationWarning): + """Indicates an API that is in 'legacy' status, a long term deprecation.""" diff --git a/releasenotes/notes/switch-to-warnings-333955d19afc99ca.yaml b/releasenotes/notes/switch-to-warnings-333955d19afc99ca.yaml new file mode 100644 index 000000000..a55c71f41 --- /dev/null +++ b/releasenotes/notes/switch-to-warnings-333955d19afc99ca.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + Warnings about deprecated behavior or deprecated/modified APIs are now + raised using the ``warnings`` module, rather than the ``logging`` module. + This allows users to filter these warnings or silence them entirely if + necessary.