Add a clear way to deprecate message schemas

Fixes: #227, #247

Signed-off-by: Aurélien Bompard <aurelien@bompard.org>

docs/messages.rst

@@ -118,3 +118,32 @@ structure for you.
 .. _the fedora-messaging repository: https://github.com/fedora-infra/fedora-messaging/tree/master/docs/sample_schema_package/
 .. _CookieCutter: https://cookiecutter.readthedocs.io
 .. _template repository: https://github.com/fedora-infra/cookiecutter-message-schemas
+
+
+
+Upgrade and deprecation
+=======================
+
+Message schema classes should not be modified in a backwards-incompatible fashion. To facilitate the
+evolution of schemas, we recommend including the schema version in the topic itself, such as
+``myservice.myevent.v1``.
+
+When a backwards-incompatible change is required, create a new class with the topic ending in
+``.v2``, set the :py:attr:`Message.deprecated` attribute to ``True`` on the old class, and send both
+versions for a reasonable period of time. Note that you need to add the new class to the schema
+package's entry points as well.
+
+We leave the duration to the developer's appreciation, since it depends on how many different
+consumers they expect to have, whether they are only inside the Fedora infrastructure or outside
+too, etc. This duration can range from weeks to months, possibly a year. At the time of this
+writing, Fedora's message bus is very far from being overwhelmed by messages, so you don't need to
+worry about that.
+
+Proceeding this way ensures that consumers subscribing to ``.v1`` will not break when ``.v2``
+arrives, and can choose to subscribe to the ``.v2`` topic when they are ready to handle the new
+format. They will get a warning in their logs when they receive deprecated messages, prompting them
+to upgrade.
+
+When you add the new version, please upgrade the major version number of your schema
+package, and communicate clearly that the old version is deprecated, including for how long you have
+decided to send both versions.

fedora_messaging/message.py

@@ -213,6 +213,16 @@ def get_message(routing_key, properties, body):
     except jsonschema.exceptions.ValidationError as e:
         _log.error("Message validation of %r failed: %r", message, e)
         raise ValidationError(e)
+
+    if MessageClass.deprecated:
+        _log.warning(
+            "A message with a deprecated schema (%s.%s) has been received on topic %r. "
+            "You should check the emitting application's documentation to upgrade to "
+            "the newer schema version.",
+            MessageClass.__module__,
+            MessageClass.__name__,
+            message.topic,
+        )
     return message
 
 
@@ -280,6 +290,10 @@ class attribute, although this is a convenient approach. Users are
         queue (str): The name of the queue this message arrived through. This
             attribute is set automatically by the library and users should never
             set it themselves.
+        deprecated (bool): Whether this message schema has been deprecated by a more
+            recent version. Emits a warning when a message of this class is received,
+            to let consumers know that they should plan to upgrade. Defaults to
+            ``False``.
     """
 
     severity = INFO
@@ -302,6 +316,7 @@ class attribute, although this is a convenient approach. Users are
         "description": "Schema for message body",
         "type": "object",
     }
+    deprecated = False
 
     def __init__(
         self, body=None, headers=None, topic=None, properties=None, severity=None

fedora_messaging/tests/unit/test_cli.py

@@ -46,6 +46,7 @@ def echo(message):
     print(str(message))
 
 
+@mock.patch("fedora_messaging.config.conf.setup_logging", mock.Mock())
 class BaseCliTests(TestCase):
     """Unit tests for the base command of the CLI."""
 
@@ -57,6 +58,7 @@ def test_no_conf(self):
 
 
 @mock.patch("fedora_messaging.cli.reactor", mock.Mock())
+@mock.patch("fedora_messaging.config.conf.setup_logging", mock.Mock())
 class ConsumeCliTests(TestCase):
     """Unit tests for the 'consume' command of the CLI."""
 
@@ -524,6 +526,7 @@ def test_file_does_not_exist(self):
         )
 
 
+@mock.patch("fedora_messaging.config.conf.setup_logging", mock.Mock())
 class PublishCliTests(TestCase):
     """Unit tests for the 'publish' command of the CLI."""
 
@@ -668,6 +671,7 @@ def test_publish_general_publish_error(self, mock_publish):
         self.assertEqual(1, result.exit_code)
 
 
+@mock.patch("fedora_messaging.config.conf.setup_logging", mock.Mock())
 class RecordCliTests(TestCase):
     """Unit tests for the 'record' command of the CLI."""
 

fedora_messaging/tests/unit/test_config.py

@@ -418,6 +418,7 @@ def test_load_on_update(self, mock_exists, mock_log):
             "Loading configuration from /etc/fedora-messaging/config.toml"
         )
 
+    @mock.patch("fedora_messaging.config.logging.config.dictConfig", mock.Mock())
     @mock.patch("fedora_messaging.config.open", mock.mock_open(read_data=empty_config))
     @mock.patch("fedora_messaging.config._log", autospec=True)
     @mock.patch("fedora_messaging.config.os.path.exists", return_value=True)

fedora_messaging/tests/unit/test_message.py

@@ -21,11 +21,16 @@
 
 import jsonschema
 import pika
+import pytest
 
 from fedora_messaging import exceptions, message
 
 
-class GetMessageTests(TestCase):
+class DeprecatedMessage(message.Message):
+    deprecated = True
+
+
+class TestGetMessage:
     """Tests for the :func:`fedora_messaging.message.get_message` function."""
 
     def test_missing_severity(self):
@@ -34,16 +39,15 @@ def test_missing_severity(self):
         del msg._headers["fedora_messaging_severity"]
 
         recv_msg = message.get_message("", msg._properties, b"{}")
-        self.assertEqual(recv_msg.severity, message.INFO)
+        assert recv_msg.severity == message.INFO
 
     def test_invalid_severity(self):
         """Assert the invalid severity fails validation."""
         msg = message.Message()
         msg._headers["fedora_messaging_severity"] = 42
 
-        self.assertRaises(
-            exceptions.ValidationError, message.get_message, "", msg._properties, b"{}"
-        )
+        with pytest.raises(exceptions.ValidationError):
+            message.get_message("", msg._properties, b"{}")
 
     def test_missing_headers(self):
         """Assert missing headers results in a default message."""
@@ -53,7 +57,27 @@ def test_missing_headers(self):
         received_msg = message.get_message(
             msg._encoded_routing_key, msg._properties, msg._encoded_body
         )
-        self.assertIsInstance(received_msg, message.Message)
+        assert isinstance(received_msg, message.Message)
+
+    @mock.patch.dict(
+        message._class_to_schema_name, {DeprecatedMessage: "deprecated_message_id"}
+    )
+    @mock.patch.dict(
+        message._schema_name_to_class, {"deprecated_message_id": DeprecatedMessage}
+    )
+    def test_deprecated(self, caplog):
+        """Assert a deprecation warning is produced when indicated."""
+        msg = DeprecatedMessage(topic="dummy.topic")
+        received_msg = message.get_message(
+            msg.topic, msg._properties, msg._encoded_body
+        )
+        assert isinstance(received_msg, DeprecatedMessage)
+        assert len(caplog.messages) == 1
+        assert caplog.messages[0] == (
+            "A message with a deprecated schema (fedora_messaging.tests.unit.test_message."
+            "DeprecatedMessage) has been received on topic 'dummy.topic'. You should check "
+            "the emitting application's documentation to upgrade to the newer schema version."
+        )
 
 
 class MessageDumpsTests(TestCase):

news/227.feature

@@ -0,0 +1 @@
+Add a message schema attribute and some documentation to help deprecate and upgrade message schemas