Speed up tip migration. Fixes #3894 #3904
Conversation
| @@ -158,19 +158,6 @@ def to_dict(self): | |||
| } | |||
|
|
|||
| def migrate_tips(self): | |||
| payment_instructions = self.db.all(""" | |||
There was a problem hiding this comment.
I think the basic filtering done above should be good enough - removing this check might do more harm than good.
We generally have 5-10 new teams every week, and ~150 teams in total. Adding the first check alone brings down the number of DB calls from 300 (150 * 2) to 20 (10 * 2). Removing this second check would bring that number down to 10 - but wouldn't give us the flexibility to blindly call Team.migrate_tips over the console. Also, the migrate-tips.py script isn't covered by our test suite, but Team.migrate_tips is.
Thoughts?
aandis
force-pushed
the
speed-up-tip-migration
branch
from
76081fb to
0707b59
Compare
| JOIN tips ON t.owner = tips.tippee -- Only fetch teams whose owner have tips. | ||
| WHERE t.is_approved IS TRUE -- Only fetch approved teams. | ||
| AND NOT EXISTS ( -- Make sure not already migrated. | ||
| SELECT 1 |
There was a problem hiding this comment.
Dammit why doesn't this align. Looks fine in the editor. 😠
There was a problem hiding this comment.
@rohitpaulk do you have any idea why the comment looks funny here but looks fine in the editor?
😡
There was a problem hiding this comment.
Most likely spaces vs tabs..
There was a problem hiding this comment.
Yep, confirmed.
This is sublime text - See the difference? The .s are spaces, and -s are tabs
There was a problem hiding this comment.
Changed it to only spaces. Should work now.
|
@rohitpaulk changes made. Also incorporated changes discussed in 3aba842 |
|
Let me know if I should undo 0707b59 |
I think it'd make sense to undo it. |
| """) | ||
|
|
||
| for slug in slugs: | ||
| team = Team.from_slug(slug) | ||
| for team in teams: | ||
| try: | ||
| ntips = team.migrate_tips() | ||
| print("Migrated {} tip(s) for '{}'".format(ntips, slug)) |
There was a problem hiding this comment.
Replace slug with team.slug
aandis
force-pushed
the
speed-up-tip-migration
branch
from
0707b59 to
fb9e25f
Compare
|
@rohitpaulk done! Since |
|
I've been running this on my local, yes - That's how I discovered #3904 (comment) :) Are you not able to run this script yourself? Is fake data completely broken? |
|
Once I have fake data in the DB, I use |
|
Thing is, my fake data doesn't have any teams which satisfy the two |
|
Works fine for me, yes - Travis seems to be complaining about something, can you fix that? |
|
Builds are passing for this pr. What is wrong? |
|
Oh |
They're still 🔴, we want 🍏! |
aandis
force-pushed
the
speed-up-tip-migration
branch
from
fb9e25f to
3a16561
Compare
|
There's your green 🍎 😛 |
|
@aandis - I've added a commit, go ahead and merge if it looks good to you |
|
http://sqlfiddle.com/#!15/20a13/5 @aandis - Is that accessible to you? |
|
Yes, Checking it out. |
|
|
Yep. Got it. #3894 described restriction on |
|
I'll merge this then. This was fun. :-) |
Speed up tip migration. Fixes #3894
|
Ok merged. Do I change the version now? What is the criteria for changing version? |
That happens automatically as part of the release process when we ship code to Heroku :) |
|
Cool! 👍 |
@rohitpaulk , @whit537 tests are failing as of now. If this looks good, I'll go ahead and fix them. ;)
#3894