1158908 - Move rbbz.models to mozreview

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect, P1)

Description

[Mark Côté [:mcote]]

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.

Comment 1

[Mark Côté [:mcote]]

Attachment: 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 a3ed77ec60bc052b0e2347a8f8ad1e7f80065124 https://reviewboard-hg.mozilla.org/version-control-tools/

Comment 2

[Mark Côté [:mcote]]

Note for local testing, you need to disable both rbbz and mozreview extensions and re-enable them after ./mozreview refresh.

Comment 3

[Gregory Szorc [:gps]]

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.

Comment 4

[Steven MacLeod [:smacleod]]

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

[Steven MacLeod [:smacleod]]

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

Comment 6

[Mark Côté [:mcote]]

https://reviewboard.mozilla.org/r/7705/#review6737

> This should really be part of the first commit.

Yeah that fix went into the wrong commit.

Comment 7

[Mark Côté [:mcote]]

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

[Steven MacLeod [:smacleod]]

https://reviewboard.mozilla.org/r/7705/#review6747

Ship It!

Comment 9

[Steven MacLeod [:smacleod]]

Comment on attachment 8598134 [details]
MozReview Request: bz://1158908/mcote

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

Ship It!

Comment 10

[Mark Côté [:mcote]]

First commit landed: http://hg.mozilla.org/hgcustom/version-control-tools/rev/77bd81d5e92f

Comment 11

[Mark Côté [:mcote]]

Second landed: http://hg.mozilla.org/hgcustom/version-control-tools/rev/1c7c9b4b7d6f

We'll be doing the data migration next.

Comment 12

[Mark Côté [:mcote]]

Deployment had some hiccups but unrelated to the migration.  Everything appears to be working.

Comment 14

[Mark Côté [:mcote]]

Attachment: MozReview Request: Bug 1158908 - mozreview: Switch BugzillaUserMap usage from rbbz to mozreview. r=smacleod

Comment 15

[Mark Côté [:mcote]]

Attachment: MozReview Request: Bug 1158908 - mozreview: Add BugzillaUserMap model to mozreview extension. r=smacleod