Closed Bug 1158908 Opened 6 years ago Closed 6 years ago

Move rbbz.models to mozreview

Categories

(MozReview Graveyard :: General, defect, P1)

Production
defect

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.
Attached file MozReview Request: bz://1158908/mcote (obsolete) —
/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)
Note for local testing, you need to disable both rbbz and mozreview extensions and re-enable them after ./mozreview refresh.
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.
Blocks: 1158513
No longer blocks: 1109721
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.
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
Attachment #8598134 - Flags: review?(smacleod)
https://reviewboard.mozilla.org/r/7705/#review6737

> This should really be part of the first commit.

Yeah that fix went into the wrong commit.
Attachment #8598134 - Flags: review?(smacleod)
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 on attachment 8598134 [details]
MozReview Request: bz://1158908/mcote

https://reviewboard.mozilla.org/r/7701/#review6749

Ship It!
Attachment #8598134 - Flags: review?(smacleod) → review+
Second landed: http://hg.mozilla.org/hgcustom/version-control-tools/rev/1c7c9b4b7d6f

We'll be doing the data migration next.
Deployment had some hiccups but unrelated to the migration.  Everything appears to be working.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #8598134 - Attachment is obsolete: true
Attachment #8620166 - Flags: review+
Attachment #8620167 - Flags: review+
Product: Developer Services → MozReview
You need to log in before you can comment on or make changes to this bug.