Colin Watson has proposed merging
~cjwatson/launchpad:py3-distribution-constructor into launchpad:master.
Commit message:
Avoid construction order problems with Distribution
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/398962
On Python 3.5, dict ordering is randomized and the order of keyword arguments
is not preserved (the latter is fixed in Python 3.6), so SQLBase.__init__ may
set arguments in any order, which can cause problems if e.g. owner validation
fails before display_name is set and thus __repr__ fails. We'll want an
explicit constructor when we convert Distribution to StormBase anyway, so do
that now to fix the order in which attributes are set and handle errors
properly.
This is made a little easier because Distribution.__init__ is only called from
DistributionSet.new.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:py3-distribution-constructor into launchpad:master.
diff --git a/lib/lp/registry/model/distribution.py b/lib/lp/registry/model/distribution.py
index 5296718..1000bea 100644
--- a/lib/lp/registry/model/distribution.py
+++ b/lib/lp/registry/model/distribution.py
@@ -278,6 +278,28 @@ class Distribution(SQLBase, BugTargetBase, MakesAnnouncements,
oci_registry_credentials = Reference(
oci_registry_credentials_id, "OCIRegistryCredentials.id")
+ def __init__(self, name, display_name, title, description, summary,
+ domainname, members, owner, registrant, mugshot=None,
+ logo=None, icon=None, vcs=None):
+ try:
+ self.name = name
+ self.display_name = display_name
+ self._title = title
+ self.description = description
+ self.summary = summary
+ self.domainname = domainname
+ self.members = members
+ self.mirror_admin = owner
+ self.owner = owner
+ self.registrant = registrant
+ self.mugshot = mugshot
+ self.logo = logo
+ self.icon = icon
+ self.vcs = vcs
+ except Exception:
+ IStore(self).remove(self)
+ raise
+
def __repr__(self):
display_name = backslashreplace(self.display_name)
return "<%s '%s' (%s)>" % (
@@ -1555,18 +1577,18 @@ class DistributionSet:
distro = Distribution(
name=name,
display_name=display_name,
- _title=title,
+ title=title,
description=description,
summary=summary,
domainname=domainname,
members=members,
- mirror_admin=owner,
owner=owner,
registrant=registrant,
mugshot=mugshot,
logo=logo,
icon=icon,
vcs=vcs)
+ IStore(distro).add(distro)
getUtility(IArchiveSet).new(distribution=distro,
owner=owner, purpose=ArchivePurpose.PRIMARY)
policies = itertools.product(
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
