Closed Bug 1264203 Opened 8 years ago Closed 8 years ago

update testing/development and production environments to use our review board/etc fork

Categories

(MozReview Graveyard :: Testing / Development Environment, defect)

Production
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

References

Details

Attachments

(9 files)

58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
once we have review board/djiblets/rbtools forked into hg.mozilla.org, we need to update our testing/development deployment scripts to use that repo.
Blocks: 1264204
Assignee: nobody → glob
Clone our Review Board fork into VCT when creating the test environment, and
ensure the local host has the tools required to package Djblets and Review
Board.  This is in preparation for using a customised Review Board instead of
an unmodified upstream version.

Review commit: https://reviewboard.mozilla.org/r/49615/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49615/
Remove Djblets and Review Board from requirements.txt, install from
reviewboard-fork instead.

Review commit: https://reviewboard.mozilla.org/r/49619/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49619/
By default when running |mozreview refresh| changes to reviewboard-fork/ will
not be reflected inside the development instance.  Specifying
--refresh-reviewboard when refreshing will also update Djblets and Review
Board, at the cost of taking twice as long to refresh.

Review commit: https://reviewboard.mozilla.org/r/49621/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49621/
Comment on attachment 8746930 [details]
MozReview Request: testing: fix pep8 issues in docker.py (bug 1264203) r=gps

https://reviewboard.mozilla.org/r/49609/#review46753

Rubber stamp. If it works, it's good enough for me :)
Attachment #8746930 - Flags: review+
Attachment #8746931 - Flags: review+
Comment on attachment 8746931 [details]
MozReview Request: testing: fix mangled output during refresh (bug 1264203) r=gps

https://reviewboard.mozilla.org/r/49611/#review46755
https://reviewboard.mozilla.org/r/49613/#review46759

::: start-local-mozreview:24
(Diff revision 1)
> +if [ -z "$(docker ps -q)" ]; then
> +    exit
> +fi

This will check for *any* running container. I think verifying the return code of `mozreview start` is a better approach. Pretty sure `mozreview start` will exit non-0 if things aren't running properly. If it doesn't, that's your bug :)
https://reviewboard.mozilla.org/r/49615/#review46761

::: create-test-environment:86
(Diff revision 1)
> +python setup.py install
> +cd ../reviewboard
> +python setup.py install

Let's use `pip` here. You'll want to pass the following arguments to get it to do the right thing:

`--upgrade --no-deps --force-reinstall`
https://reviewboard.mozilla.org/r/49617/#review46765

::: testing/vcttesting/docker.py:218
(Diff revision 1)
> -        args = [hg, '-R', ROOT, 'locate']
> +        args = [hg, '-R', '.', 'locate']
>          with open(os.devnull, 'wb') as null:
> -            output = subprocess.check_output(args, env=env, cwd='/',
> -                                             stderr=null)
> +            files = subprocess.check_output(
> +                args, env=env, cwd=ROOT, stderr=null).splitlines()

My spider senses tell me there was a good reason why we ran cwd from /. I think it has something to do with picking up configs it isn't supped to based on the cwd. You may want to look at blame for this code for the history.
https://reviewboard.mozilla.org/r/49619/#review46767

::: ansible/roles/docker-rbweb/tasks/main.yml:19
(Diff revision 1)
>  
>  - name: Synchronize version-control-tools
>    synchronize: src={{ vct }}/ dest=/version-control-tools/ recursive=yes delete=yes
>  
> +- name: Install Djblets and Review Board
> +  command: /venv/bin/python setup.py install chdir=/version-control-tools/reviewboard-fork/{{ item }}

Again, use pip with the arguments I specified in the other review.
Comment on attachment 8746936 [details]
MozReview Request: testing: add --refresh-reviewboard to |mozreview refresh| (bug 1264203) r=gps

https://reviewboard.mozilla.org/r/49621/#review46769
Attachment #8746936 - Flags: review+
https://reviewboard.mozilla.org/r/49623/#review46771

Can you make it refresh reviewboard automation if reviewboard changes? This feels much more user intuitive. You may need to introduce an intermediate process that queries watchman for files that changed since last time and does the appropriate refresh.
https://reviewboard.mozilla.org/r/49617/#review46765

> My spider senses tell me there was a good reason why we ran cwd from /. I think it has something to do with picking up configs it isn't supped to based on the cwd. You may want to look at blame for this code for the history.

cwd='/' has existed since the initial commit (bug 1090575).

> I think it has something to do with picking up configs it isn't supped to based on the cwd

it may have in the past, but currently mercurial doesn't look at the cwd when finding config files:

> On Unix, the following files are consulted:
>   - "<repo>/.hg/hgrc" (per-repository)
>   - "$HOME/.hgrc" (per-user)
>   - "<install-root>/etc/mercurial/hgrc" (per-installation)
>   - "<install-root>/etc/mercurial/hgrc.d/*.rc" (per-installation)
>   - "/etc/mercurial/hgrc" (per-system)
>   - "/etc/mercurial/hgrc.d/*.rc" (per-system)
>   - "<internal>/default.d/*.rc" (defaults)

i think we're good here, but i can change it if you still carry concerns (using cwd=ROOT simplifies things slightly).
https://reviewboard.mozilla.org/r/49615/#review46761

> Let's use `pip` here. You'll want to pass the following arguments to get it to do the right thing:
> 
> `--upgrade --no-deps --force-reinstall`

this broke rb-site: pkg_resources.DistributionNotFound: The 'Djblets<=0.9.999,>=0.9.3' distribution was not found and is required by ReviewBoard
i have confirmed that djblets is installed within the venv.

in order to determine if i should pour time into fixing this, what's the advantage of using pip here? (note that none of our other packages are installed using pip)
https://reviewboard.mozilla.org/r/49615/#review46761

> this broke rb-site: pkg_resources.DistributionNotFound: The 'Djblets<=0.9.999,>=0.9.3' distribution was not found and is required by ReviewBoard
> i have confirmed that djblets is installed within the venv.
> 
> in order to determine if i should pour time into fixing this, what's the advantage of using pip here? (note that none of our other packages are installed using pip)

.. this only happens within the docker container; pip install works fine within the host venv.
Summary: update testing/development environment to use our review board/etc fork → update testing/development and production environments to use our review board/etc fork
Review commit: https://reviewboard.mozilla.org/r/52113/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52113/
Attachment #8746930 - Attachment description: MozReview Request: testing: fix pep8 issues in docker.py (bug 1264203) → MozReview Request: testing: fix pep8 issues in docker.py (bug 1264203) r=gps
Attachment #8746931 - Attachment description: MozReview Request: testing: fix mangled output during refresh (bug 1264203) → MozReview Request: testing: fix mangled output during refresh (bug 1264203) r=gps
Attachment #8746932 - Attachment description: MozReview Request: testing: start-local-mozreview shouldn't continue if docker failed to start (bug 1264203) → MozReview Request: testing: start-local-mozreview shouldn't continue if docker failed to start (bug 1264203) r?gps
Attachment #8746933 - Attachment description: MozReview Request: testing: clone reviewboard-fork when starting mozreview (bug 1264203) → MozReview Request: testing: clone reviewboard-fork when starting mozreview (bug 1264203) r?gps
Attachment #8746934 - Attachment description: MozReview Request: testing: sync reviewboard-fork into vct container (bug 1264203) → MozReview Request: testing: sync reviewboard-fork into vct container (bug 1264203) r?gps
Attachment #8746935 - Attachment description: MozReview Request: testing: switch dev/test environment from release egg to fork deployment (bug 1264203) → MozReview Request: testing: switch dev/test environment from release egg to fork deployment (bug 1264203) r?gps
Attachment #8746936 - Attachment description: MozReview Request: testing: add --refresh-reviewboard to |mozreview refresh| (bug 1264203) → MozReview Request: testing: add --refresh-reviewboard to |mozreview refresh| (bug 1264203) r=gps
Attachment #8746937 - Attachment description: MozReview Request: testing: add autorefresh/watchman support for --refresh-reviewboard (bug 1264203) → MozReview Request: testing: add autorefresh/watchman support for --refresh-reviewboard (bug 1264203) r?gps
Attachment #8751584 - Flags: review?(gps)
Attachment #8746932 - Flags: review?(gps)
Attachment #8746933 - Flags: review?(gps)
Attachment #8746934 - Flags: review?(gps)
Attachment #8746935 - Flags: review?(gps)
Attachment #8746937 - Flags: review?(gps)
Comment on attachment 8746930 [details]
MozReview Request: testing: fix pep8 issues in docker.py (bug 1264203) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49609/diff/1-2/
Comment on attachment 8746931 [details]
MozReview Request: testing: fix mangled output during refresh (bug 1264203) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49611/diff/1-2/
Comment on attachment 8746932 [details]
MozReview Request: testing: start-local-mozreview shouldn't continue if docker failed to start (bug 1264203) r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49613/diff/1-2/
Comment on attachment 8746933 [details]
MozReview Request: testing: clone reviewboard-fork when starting mozreview (bug 1264203) r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49615/diff/1-2/
Comment on attachment 8746934 [details]
MozReview Request: testing: sync reviewboard-fork into vct container (bug 1264203) r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49617/diff/1-2/
Comment on attachment 8746935 [details]
MozReview Request: testing: switch dev/test environment from release egg to fork deployment (bug 1264203) r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49619/diff/1-2/
Comment on attachment 8746936 [details]
MozReview Request: testing: add --refresh-reviewboard to |mozreview refresh| (bug 1264203) r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49621/diff/1-2/
Comment on attachment 8746937 [details]
MozReview Request: testing: add autorefresh/watchman support for --refresh-reviewboard (bug 1264203) r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49623/diff/1-2/
Comment on attachment 8746932 [details]
MozReview Request: testing: start-local-mozreview shouldn't continue if docker failed to start (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/49613/#review49121
Attachment #8746932 - Flags: review?(gps) → review+
Attachment #8746933 - Flags: review?(gps) → review+
Comment on attachment 8746933 [details]
MozReview Request: testing: clone reviewboard-fork when starting mozreview (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/49615/#review49125

::: create-test-environment:91
(Diff revision 2)
> +python setup.py install
> +cd ../reviewboard
> +python setup.py install

Do you want to run `setup.py develop` so the virtualenv runs out of the source checkout? That way if you modify sources they should get picked up without any more `setup.py` needed.
Attachment #8746934 - Flags: review?(gps) → review+
Comment on attachment 8746934 [details]
MozReview Request: testing: sync reviewboard-fork into vct container (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/49617/#review49127
Comment on attachment 8746935 [details]
MozReview Request: testing: switch dev/test environment from release egg to fork deployment (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/49619/#review49129
Attachment #8746935 - Flags: review?(gps) → review+
Comment on attachment 8746937 [details]
MozReview Request: testing: add autorefresh/watchman support for --refresh-reviewboard (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/49623/#review49133
Attachment #8746937 - Flags: review?(gps) → review+
Attachment #8751584 - Flags: review?(gps) → review+
Comment on attachment 8751584 [details]
MozReview Request: deployment: update production deployment to for rb-fork changes (bug 1264203) r?gps

https://reviewboard.mozilla.org/r/52113/#review49135
https://reviewboard.mozilla.org/r/49615/#review49125

> Do you want to run `setup.py develop` so the virtualenv runs out of the source checkout? That way if you modify sources they should get picked up without any more `setup.py` needed.

that's a good question, and i'll experiment, but i'm not sure that's needed.

as far as i can tell reviewboard is only deployed into the host venv because it's a prerequisite for our extensions, parts of which are used to communicate with bugzilla/etc running inside docker.

that said, if nothing explodes with 'develop' no harm in doing it.
Pushed by bjones@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/153d845a19ea
testing: fix pep8 issues in docker.py r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/aabadfd4aa55
testing: fix mangled output during refresh r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/7295c7cebabc
testing: start-local-mozreview shouldn't continue if docker failed to start r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/f1cc4b5a92b1
testing: clone reviewboard-fork when starting mozreview r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/6ac06979f506
testing: sync reviewboard-fork into vct container r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/94abb1e0519c
testing: switch dev/test environment from release egg to fork deployment r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/0386aef9c798
testing: add --refresh-reviewboard to |mozreview refresh| r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/4816c7f440e1
testing: add autorefresh/watchman support for --refresh-reviewboard r=gps
https://hg.mozilla.org/hgcustom/version-control-tools/rev/65377d4d2133
deployment: update production deployment to for rb-fork changes r=gps
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: