-
Notifications
You must be signed in to change notification settings - Fork 722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bug 971014] Upgrade to django 1.6 #1877
Conversation
@@ -889,3 +881,6 @@ def read_only_mode(env): | |||
AXES_COOLOFF_TIME = 1 # hour | |||
AXES_BEHIND_REVERSE_PROXY = True | |||
AXES_REVERSE_PROXY_HEADER = 'HTTP_X_CLUSTER_CLIENT_IP' | |||
|
|||
# Set this to True to wrap each HTTP request in a transaction on this database. | |||
ATOMIC_REQUESTS = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, I'm pretty sure this is the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's nice to have here as a reminder that we're doing ATOMIC_REQUESTS.
When I run the tests, I get this:
If I run
|
whoa! I only ran with FORCE_DB. I'm curious to see what travis says. |
I can make the "no module named defaults" ones go away by updating django-adminplus to 0.2.1. That unearths lots of errors like this:
I think that suggests django-adminplus changed. I didn't look into this further. Running without FORCE_DB brings up the issue that the mysql backend no longer has that "commit_unless_managed" thing. There's one instance of commit_unless_managed, and it says its a no-op: https://github.com/django/django/blob/master/django/db/transaction.py#L122 I think this means we need to fix Hope that helps! |
OK, tests are passing for me now after pyc cleanup and all ^^ |
I'm looking at this now.... |
Tests pass when I run with
|
GAH! Forgot to look at that one. |
It only pulls in settings_test.py
OK, this time I ran with and without FORCE_DB... Do I dare ask for a r? |
def setup_test_environment(self, **kwargs): | ||
# If we have a settings_test.py let's roll it into our settings. | ||
try: | ||
import settings_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would totally take the opportunity here to put settings_test.py
in the right place. I.e. put it in kitsune/settings_test.py
and import it with from kitsune import settings_test
. Then we have one less file in the root dir and all our settings together!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent idea!
moved settings_test.py ^ |
Tests pass now. Everything looks good to me. What's the plan for deploying this? Will we push it to stage, run the QA stuff and then go from there? Do we want this to sit on stage over the weekend? |
afa2b06 [bug 971014] Upgrade to django 1.6 |
Well, I think this is it!
_lazy
to beunicode
.ATOMIC_REQUESTS = True
. So I did that.r?