Vagrant config and Makefile improvements #3
Conversation
|
@brousch Thank you for the PR! That looks like a very useful contribution to run this, especially for people not using Ubuntu or Debian. I'll schedule a review for it in our next sprint, ie likely next week. The build seem to fail because of missing auth, I think this is because the PR is from a fork - I'll need to look into this, not sure if there is a secure way to handle these cases without making the auth potentially publicly available. |
| @@ -1,6 +1,16 @@ | |||
| # Config | |||
| WORKERS = 4 | |||
| SHELL = /bin/bash | |||
| MANAGE := python ./manage.py | |||
There was a problem hiding this comment.
@brousch There isn't really a need to call python explicitly here - it duplicates the shebang from manage.py. I would only keep HONCHO_MANAGE := honcho run ./manage.py
There was a problem hiding this comment.
Oh, I missed your comment on this:
Line 5: honcho run ./manage.py is not reliable when manage.py is not executable, which can happen when using Vagrant on Windows. Using the full honcho run python ./manage.py everywhere avoids this issue.
Sounds good then. Maybe add a comment in the Makefile to ensure people understand why this is necessary?
|
@brousch Thanks again for this PR! It's going to be very useful to add vagrant support. |
|
Re-run the tests from a branch from the main repo to get the auth - it passed correctly. @brousch when you will push new commits to your branch, it will fail again due to the missing auth on your fork, but I'll run them again when I re-review later. |
# This is the 1st commit message: Re-writing circle.yml to store code coverage in CircleCI's workspace and collect them afterwards. Test run parallelization is used with environment variables instead of using custom test runner. # This is the commit message #2: Update Makefile message. # This is the commit message #3: Remove cov.html from cov.combine.
# This is the 1st commit message: Re-writing circle.yml to store code coverage in CircleCI's workspace and collect them afterwards. Test run parallelization is used with environment variables instead of using custom test runner. # This is the commit message #2: Update Makefile message. # This is the commit message #3: Remove cov.html from cov.combine. # This is the commit message #4: Stop running all tests in all containers # This is the commit message #5: Cleanup
# This is the 1st commit message: Re-writing circle.yml to store code coverage in CircleCI's workspace and collect them afterwards. Test run parallelization is used with environment variables instead of using custom test runner. # This is the commit message #2: Update Makefile message. # This is the commit message #3: Remove cov.html from cov.combine. # This is the commit message #4: Stop running all tests in all containers # This is the commit message #5: Cleanup # This is the commit message #6: Experiment with different IDs for each coverage report # This is the commit message #7: Gathering raw coverage from all steps # This is the commit message #8: Always copy coverage and log message when not found # This is the commit message #9: Log copying raw coverage files for debugging
These are changes to allow a dev instance to be run via Vagrant. some new Makefile commands are included and the README has been updated to reflect the changes. Some changes that may raise eyebrows are:
.env.vagrant:
I'm not sure about all of the settings in the .env file, but this is sufficient for running a dev instance using SQLite.
Line 3: The DB needs to be kept out of the repo due to Windows issues in Vagrant if it's in the repo.
Makefile:
Line 5: honcho run ./manage.py is not reliable when manage.py is not executable, which can happen when using Vagrant on Windows. Using the full honcho run python ./manage.py everywhere avoids this issue.
Line 9: If they are running Vagrant on Windows, line endings can be a problem here. The new code here strips off the troublesome ^M should it appear.
Procfile.dev:
Line 3: We cannot expose only 127.0.0.1:5000 because the port will not be forwarded to the host in Vagrant, so we run on 0.0.0.0:5000
Vagrantfile:
Line 25: Port 80 is forwarded in anticipation of production working, thought this is not yet complete.
opencraft/settings.py:
Line 39: This is really a bug fix. The instructions mention that you can use a .env file, but the necessary code for reading that file was not present.