Merge ~d0ugal/maas:release-notification-setting into maas:master

[Dougal Matthews]

Add the Release Notification configuration option

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release-notification-setting lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8030/console
COMMIT: 2977a161338b30ff052b353bac13d8ab138bb334

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release-notification-setting lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8031/console
COMMIT: bdd765e5f11bcb8aa0e3d17d27eb63e7d90676b2

review: Needs Fixing

[Lee Trager (ltrager)]

Why are we making this a configuration option? None of our other notifications are optional. The data will be coming from the MAAS streams so it will be downloaded regardless.

review: Needs Information

[Kit Randel (blr)]

That was the design agreed upon
https://app.zeplin.io/project/5efa65801b4d9162739308fc/screen/5f16b3b5311acb77fa0925d7

On Tue, Jul 28, 2020 at 12:38 PM Lee Trager <email address hidden>
wrote:

> Review: Needs Information
>
> Why are we making this a configuration option? None of our other
> notifications are optional. The data will be coming from the MAAS streams
> so it will be downloaded regardless.
> --
> https://code.launchpad.net/~d0ugal/maas/+git/maas/+merge/388137
> Your team MAAS Committers is subscribed to branch maas:master.
>
> Launchpad-Message-Rationale: Subscriber @maas-committers
> Launchpad-Message-For: maas-committers
> Launchpad-Notification-Type: code-review
> Launchpad-Branch: ~maas-committers/maas/+git/maas:master
> Launchpad-Project: maas
>

--
Kit Randel (blr)
Canonical - Web Team

[Kit Randel (blr)]

Also described in the new release notification spec

[Kit Randel (blr)]

Dougal, do you have a followup branch planned to prevent sending release notifications if this config is enabled?

[Dougal Matthews (d0ugal)]

> Why are we making this a configuration option? None of our other notifications
> are optional. The data will be coming from the MAAS streams so it will be
> downloaded regardless.

Right, this is exactly as Kit mentioned. This is a requirement of the release notification work, we want to give admins the ability to disable it.

[Dougal Matthews (d0ugal)]

> Dougal, do you have a followup branch planned to prevent sending release
> notifications if this config is enabled?

I have a follow up branch which I am working on now that will use this config option and send or not send the notifications (nothing creates them yet). I wanted to get this change ahead of the rest so that any UI work is unblocked.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release-notification-setting lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 4ee0c6ba7fc0959adaebaee3b42688fa0523c421

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release-notification-setting lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ce001452d24899e5a2b9a33979d156ccb2efe114

review: Approve

[Lee Trager (ltrager)]

I understand you're using a list so future notification types can be toggled. The Config model has API and websocket code so new values should just work. As this is a new type you may need to modify those as well. I wonder if we should keep this as a simple boolean as that is what the design calls for. In the future we could create new boolean config values as well if need be.

review: Needs Information

[Dougal Matthews (d0ugal)]

> I understand you're using a list so future notification types can be toggled.
> The Config model has API and websocket code so new values should just work. As
> this is a new type you may need to modify those as well. I wonder if we should
> keep this as a simple boolean as that is what the design calls for. In the
> future we could create new boolean config values as well if need be.

Thanks, that is a valid concern. To save time I have switched back to a boolean for now.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b release-notification-setting lp:~d0ugal/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8043/console
COMMIT: f11665f214ab20b0f2cb7b874cdb99014255e7f2

review: Needs Fixing

[Lee Trager (ltrager)]

LGTM!

review: Approve

618e33a... by Dougal Matthews on 2020-07-31

Blacken code

diff --git a/src/maasserver/forms/settings.py b/src/maasserver/forms/settings.py
index f03eb04..f2d5692 100644
--- a/src/maasserver/forms/settings.py
+++ b/src/maasserver/forms/settings.py
@@ -753,6 +753,16 @@ CONFIG_ITEMS = {
             "min_value": 1,
         },
     },
+    "release_notifications": {
+        "default": True,
+        "form": forms.BooleanField,
+        "form_kwargs": {
+            "required": False,
+            "label": (
+                "Enable or disable notifications for new MAAS releases."
+            ),
+        },
+    },
     "use_rack_proxy": {
         "default": True,
         "form": forms.BooleanField,
diff --git a/src/maasserver/models/config.py b/src/maasserver/models/config.py
index 2aa381d..b6e9d9e 100644
--- a/src/maasserver/models/config.py
+++ b/src/maasserver/models/config.py
@@ -105,6 +105,7 @@ def get_default_config():
         "max_node_installation_results": 3,
         # Notifications.
         "subnet_ip_exhaustion_threshold_count": 16,
+        "release_notifications": True,
         # Authentication.
         "external_auth_url": "",
         "external_auth_user": "",