launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/allow-none-in-snapbuild-metadata into lp:launchpad.

Commit message:
Set the metadata to {} on each access

Requested reviews:
  Colin Watson (cjwatson)

For more details, see:
https://code.launchpad.net/~twom/launchpad/allow-none-in-snapbuild-metadata/+merge/361089
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-12-18 17:20:04 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-12-18 18:21:05 +0000
@@ -175,9 +175,7 @@
 
     failure_count = Int(name='failure_count', allow_none=False)
 
-    # Private variable that is used by the `store_upload_metadata` to
-    # set as an empty dict rather than None, to handle pre-existing data.
-    _store_upload_metadata = JSON('store_upload_json_data', allow_none=True)
+    store_upload_metadata = JSON('store_upload_json_data', allow_none=True)
 
     def __init__(self, build_farm_job, requester, snap, archive,
                  distro_arch_series, pocket, channels, processor, virtualized,
@@ -194,22 +192,12 @@
         self.processor = processor
         self.virtualized = virtualized
         self.date_created = date_created
-        self._store_upload_metadata = store_upload_metadata
+        self.store_upload_metadata = store_upload_metadata
         if build_request is not None:
             self.build_request_id = build_request.id
         self.status = BuildStatus.NEEDSBUILD
 
     @property
-    def store_upload_metadata(self):
-        if self._store_upload_metadata is None:
-            self._store_upload_metadata = {}
-        return self._store_upload_metadata
-
-    @store_upload_metadata.setter
-    def store_upload_metadata(self, value):
-        self._store_upload_metadata = value
-
-    @property
     def build_request(self):
         """See `ISnapBuild`."""
         if self.build_request_id is not None:

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2018-12-18 17:15:21 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2018-12-18 18:21:05 +0000
@@ -216,7 +216,7 @@
         """See `ISnapStoreUploadJob`."""
         intermediate = {}
         intermediate.update(self.metadata)
-        intermediate.update(self.snapbuild.store_upload_metadata)
+        intermediate.update(self.snapbuild.store_upload_metadata or {})
         return intermediate
 
     @property
@@ -257,6 +257,8 @@
     @store_url.setter
     def store_url(self, url):
         """See `ISnapStoreUploadJob`."""
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["store_url"] = url
 
     @property
@@ -267,6 +269,8 @@
     @store_revision.setter
     def store_revision(self, revision):
         """See `ISnapStoreUploadJob`."""
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["store_revision"] = revision
 
     @property
@@ -276,6 +280,8 @@
 
     @status_url.setter
     def status_url(self, url):
+        if self.snapbuild.store_upload_metadata is None:
+            self.snapbuild.store_upload_metadata = {}
         self.snapbuild.store_upload_metadata["status_url"] = url
 
     # Ideally we'd just override Job._set_status or similar, but