Closed
Bug 1158908
Opened 7 years ago
Closed 7 years ago
Move rbbz.models to mozreview
Categories
(MozReview Graveyard :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mcote, Unassigned)
References
Details
Attachments
(2 files, 1 obsolete file)
Moving the BugzillaUserMap model out of rbbz and into mozreview is necessary to avoid circular dependencies in bug 1158513. However, this is a bit tricky, since the database table names include the name of the extension, e.g. rbbz_bugzillausermap. We'll probably need a tiny bit of downtime to do the switchover: 1. Deploy a commit with the model copied over to mozreview and no other changes. This will create the requisite table in the db. 2. Copy all the data from rbbz_bugzillausermap to mozreview_bugzillausermap. 3. Deploy a second commit that changes all the imports to mozreview.models.BugzillaUserMap and removes rbbz/models.py.
Assignee | ||
Comment 1•7 years ago
|
||
/r/7703 - Bug 1158908 - mozreview: Add BugzillaUserMap model to mozreview extension. r=smacleod /r/7705 - Bug 1158908 - mozreview: Switch BugzillaUserMap usage from rbbz to mozreview. r=smacleod Pull down these commits: hg pull -r a3ed77ec60bc052b0e2347a8f8ad1e7f80065124 https://reviewboard-hg.mozilla.org/version-control-tools/
Attachment #8598134 -
Flags: review?(smacleod)
Assignee | ||
Comment 2•7 years ago
|
||
Note for local testing, you need to disable both rbbz and mozreview extensions and re-enable them after ./mozreview refresh.
Comment 3•7 years ago
|
||
https://reviewboard.mozilla.org/r/7705/#review6591 There is a reference to rbbz.models.BugzillaUserMap in ansible/roles/docker-rbweb/files/install-reviewboard. This partially accounts for why `./mozreview refresh` doesn't work with this patch.
Assignee | ||
Updated•7 years ago
|
Comment 4•7 years ago
|
||
https://reviewboard.mozilla.org/r/7703/#review6717 Fix-it then ship-it assuming that everything works when you remove the import from extension.py ::: pylib/mozreview/mozreview/extension.py:19 (Diff revision 1) > +from mozreview.bugzilla.models import BugzillaUserMap We shouldn't be importing this here and not using it. Are you seeing that this is required for some reason? Review Board should be picking up the model by automatically importing the extensions top level model.py ::: pylib/mozreview/mozreview/models.py:10 (Diff revision 1) > + 'BugzillaUserMap' Trailing comma in lists like this please. ::: pylib/mozreview/mozreview/models.py:8 (Diff revision 1) > 'AutolandRequest', > - 'AutolandEventLogEntry' > + 'AutolandEventLogEntry', > + 'BugzillaUserMap' Can you alphabetize these while you're here.
Comment 5•7 years ago
|
||
https://reviewboard.mozilla.org/r/7705/#review6721 ::: pylib/rbbz/rbbz/extension.py:9 (Diff revision 1) > from django.contrib.sites.models import Site > from django.db.models.signals import pre_delete > - > from djblets.siteconfig.models import SiteConfiguration > from djblets.util.decorators import simple_decorator even though the blank line isn't technically PEP8 (it breaks up the third party section so Review Board core stuff is special) I still kind of prefer it. go with what makes sense to you though. ::: pylib/rbbz/rbbz/auth.py:12 (Diff revision 1) > +from mozreview.models import get_or_create_bugzilla_users I think we should keep the mozreview imports with the rbbz ones like they are part of the same package - they will be eventually and it's nice to have the grouping of what we control. (Also see my other comment about blank space between django and RB stuff) ::: pylib/mozreview/mozreview/extension.py (Diff revision 1) > -from mozreview.bugzilla.models import BugzillaUserMap This should really be part of the first commit. ::: pylib/mozreview/mozreview/bugzilla/models.py:19 (Diff revision 1) > class BugzillaUserMap(models.Model): Like gps said, there are instances of BugzillaUserMap use that this patch misses. I'll open this issue to track it. See: https://dxr.mozilla.org/hgcustom:version-control-tools/search?q=BugzillaUserMap&case=true&=hgcustom%3Aversion-control-tools&redirect=true
Updated•7 years ago
|
Attachment #8598134 -
Flags: review?(smacleod)
Assignee | ||
Comment 6•7 years ago
|
||
https://reviewboard.mozilla.org/r/7705/#review6737 > This should really be part of the first commit. Yeah that fix went into the wrong commit.
Assignee | ||
Updated•7 years ago
|
Attachment #8598134 -
Flags: review?(smacleod)
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8598134 [details] MozReview Request: bz://1158908/mcote /r/7703 - Bug 1158908 - mozreview: Add BugzillaUserMap model to mozreview extension. r=smacleod /r/7705 - Bug 1158908 - mozreview: Switch BugzillaUserMap usage from rbbz to mozreview. r=smacleod Pull down these commits: hg pull -r 7571acf66649ee0db3a65bf962fe97e78c55ab51 https://reviewboard-hg.mozilla.org/version-control-tools/
Comment 8•7 years ago
|
||
https://reviewboard.mozilla.org/r/7705/#review6747 Ship It!
Comment 9•7 years ago
|
||
Comment on attachment 8598134 [details] MozReview Request: bz://1158908/mcote https://reviewboard.mozilla.org/r/7701/#review6749 Ship It!
Attachment #8598134 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 10•7 years ago
|
||
First commit landed: http://hg.mozilla.org/hgcustom/version-control-tools/rev/77bd81d5e92f
Assignee | ||
Comment 11•7 years ago
|
||
Second landed: http://hg.mozilla.org/hgcustom/version-control-tools/rev/1c7c9b4b7d6f We'll be doing the data migration next.
Assignee | ||
Comment 12•7 years ago
|
||
Deployment had some hiccups but unrelated to the migration. Everything appears to be working.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•7 years ago
|
||
Attachment #8598134 -
Attachment is obsolete: true
Attachment #8620166 -
Flags: review+
Attachment #8620167 -
Flags: review+
Assignee | ||
Comment 14•7 years ago
|
||
Assignee | ||
Comment 15•7 years ago
|
||
Updated•6 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•