1060463 - check requirements file vs. virtual environment

Status: RESOLVED FIXED

(Webtools Graveyard :: DXR, defect)

Description

[Will Kahn-Greene [:willkg] ET needinfo? me]

The conversation was in pr #314. Relevant bits:

@willkg said:

    I had some problems switching branches and dealing with requirements 
    in the vagrant environment. With fjord, we create a virtual environment 
    and then activate it in .bashrc when you log in. Here, it looks like 
    everything is installed at the system level, so when I go to pick up 
    pyelasticsearch which is a new requirement in this or a related branch,
    it gets a little fussy.

    When you switch branches are you rebuilding your vagrant vm? Is there 
    a good way for dealing with requirements changing between branches?

Then @erikrose thoughtfully replied:

    I like the venv idea! Right now, you have to use sudo to get anywhere 
    installing requirements. (And yes, you do have to either throw away 
    your vagrant box or manually run sudo ./peep.py install -r 
    requirements.txt each time you switch to a branch with different 
    requirements. We haven't historically had many of these, so we haven't 
    felt the pain to motivate us to fix it.)

    The venv thing would be an easy, welcome improvement. I'm less sure 
    how to make switching branches lower-touch.

Then @willkg offered in an offhanded fashion:

    I'll look at adding a venv patch. In Fjord we do this fancy thing 
    with manage.py where it compares what you've got installed with the 
    requirements file and tells you if things are out of sync.

    https://github.com/mozilla/fjord/blob/master/fjord/manage_utils.py#L21

    We could kick that off in dxr-serve.py or somewhere else that devs are 
    likely to use but isn't in use in prod.

    Want me to create a bug for this to track it (and move the conversation 
    to)?

To which @erikrose stated in a calm manner:

    Actually, we could attach that warning of yours to dxr-serve, and all 
    would be fine. That's meant to be used just for development; only a 
    crazy person would deploy with it.


This bug covers adding a thing like fjord has to dxr and having it kick off in dxr-serve.

Comment 1

[Will Kahn-Greene [:willkg] ET needinfo? me]

Is this relevant anymore? I haven't touched dxr in a while, so I'm not sure.

Comment 2

[Erik Rose [:erik][:erikrose]]

We do use a venv on DXR's vagrant box now, so that bit is done. However, if someone wanted to teach dxr-serve to compare requirements with what's installed, they could. But, as time has passed, I have an idea that might be better:

We should add a make target called "requirements" that calls peep. It would depend on requirements.txt and lay down a .requirements_installed file, akin to the .npm_installed rule, so it never hits the network (or takes any time) unless the requirements have actually been changed since they were installed. Then we have "make all" depend on the "requirements" target. We also replace the direct call to peep in deploy.py with "make requirements" for DRY purposes. Then we just continue telling people what I've been telling them: run make all the time. Something not working? Run make. Run it! :-) This way, we take care of not just Python requirements but node ones and cachebusting, all with just one incantation to remember.

Comment 3

[[github robot]]

Commit pushed to master at https://github.com/mozilla/dxr

https://github.com/mozilla/dxr/commit/a7daaaaad2d0f8ed54c0a5385347b597489463f8
Make make responsible for installing python requirements. Fixes bug #1060463.

This gives contribs who are new to Python a fighting chance to get their requirements updated without bothering me: when they run make.

For now, uninstalling the Python requirements isn't included in `make clean`. Development may or may not take place inside the vagrant box, and, if it isn't, wiping out somebody's venv (hopefully it's a venv!) would be surprising.

Also...
* Modernize setup.py develop call to a pip call.
* Remove a stray reference to the deleted "templates" make target.
* Requirements no longer get installed during vagrant provisioning. They'll get installed when make is run.