Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Fix bug 1065624: Remove subscriber data from db. #141

Closed
wants to merge 3 commits into from

Conversation

pmclanahan
Copy link

This relies on #140 and includes the commit from there.

@pmclanahan
Copy link
Author

@jgmize this is a big step on the way to the move away from generic cluster. if you get some time I'd appreciate at least some first pass thoughts.

@pmclanahan
Copy link
Author

NOTE: the move from django-nose to pytest-django is because the former was giving me fits. pytest seems much more well maintained at least as far as django integration is concerned.

@@ -119,6 +119,7 @@ def on_failure(self, exc, task_id, args, kwargs, einfo):
The return value of this handler is ignored.

"""
print 'CALLED on_failure!!!!!!!!!!'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was probably left in accidentally?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed. oops. good catch.

@jgmize
Copy link

jgmize commented Jul 24, 2015

Overall this this is looking good so far on visual inspection, but I'd like to get together and discuss testing and deployment plans. Are you wanting to test and deploy #140 first and then tackle this?

@@ -7,22 +7,27 @@
unsubscribe, user)


def token_url(url_prefix, *args, **kwargs):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgmize this look okay? I simplified it a bit more than you'd suggested.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 even better!

@pmclanahan
Copy link
Author

I do think #140 should go first, however, we're still not on the new Py2.7 cluster for stage and prod. So this will need to stay here until that's done at least.

After this I've got a bit more to do to get it heroku-ready, then I'll do an attempt at deploying elsewhere.

@pmclanahan
Copy link
Author

Maybe I should resubmit this as a PR against the django-1.7 branch...

@pmclanahan
Copy link
Author

I've created the 12factor branch and want to have this as a PR against that, but I don't see a way to modify this PR in that way. I guess I'll have to close this one and open another.

@pmclanahan
Copy link
Author

Closing in favor of #142.

@pmclanahan pmclanahan closed this Jul 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants