Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

Descendant karma attribute no longer disappears off python objects. #94

Merged
merged 5 commits into from Mar 25, 2014
Merged

Descendant karma attribute no longer disappears off python objects. #94

merged 5 commits into from Mar 25, 2014

Conversation

LucasSloan
Copy link
Contributor

Currently when comments age out of cache, their descendant karma disappears. Now when they're rebuilt from the database, it gets set.

@LucasSloan LucasSloan mentioned this pull request Dec 9, 2013
Open
jack-trikeapps
jack-trikeapps reviewed Dec 10, 2013
View reviewed changes
r2/r2/lib/db/tdb_sql.py
sa.Integer,
default = 0,
nullable = False))
if name not in ('comment', 'link'):
Copy link
Contributor

Choose a reason for hiding this comment

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

In the comments for the postmovement pull request, you mentioned that deduplicating and using append_column "didn't work". I've been poking around the sqlalchemy source and it appears that all of the columns being passed to the constructor are set up in the same way as the column added with append_column. (See _init_items and append_column in lib/sqlalchemy/schema.py). Can you investigate further, find out what exactly is breaking when you revert to the old version (using append_column). If you have trouble, get back to me with a test case of something that breaks and I'll take a look too.

@LucasSloan
Copy link
Contributor Author

Requested changes implemented.

jack-trikeapps
jack-trikeapps reviewed Mar 24, 2014
View reviewed changes
r2/r2/lib/db/tdb_sql.py
sa.DateTime(timezone = True),
default = sa.func.now(),
nullable = False))

Copy link
Contributor

Choose a reason for hiding this comment

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

How did these shift all shift four spaces in? I can't see any signs of a tab/space mixup or anything, but it'd be good to have them lined up neatly. Besides, it'll keep the diff of the branch as a whole tidy,

@jack-trikeapps
Copy link
Contributor

Good to hear from you again. I've looked at the changes on this PR - just a
little whitespace fix, then I'll test and hopefully merge tomorrow.

-- Jack

On Mon, Mar 24, 2014 at 2:58 PM, Lucas Sloan notifications@github.comwrote:

Requested changes implemented.

Reply to this email directly or view it on GitHubhttps://github.com//pull/94#issuecomment-38410665
.

@jack-trikeapps
Copy link
Contributor

How would I go about testing this change? What behaviours should I be expecting?

@LucasSloan
Copy link
Contributor Author

More or less, the changes here are invisible - that's why I didn't notice
the problem the first time. But if you know how to drop all of the caches,
that would work. I did some hamfisted things to do it. Basically, almost
all the objects are kept in cache, not rebuilt from the db every time.
This change makes sure the rebuild works.

On Mon, Mar 24, 2014 at 3:52 PM, jack-trikeapps notifications@github.comwrote:

How would I go about testing this change? What behaviours should I be
expecting?

Reply to this email directly or view it on GitHubhttps://github.com//pull/94#issuecomment-38511672
.

"And someday when the descendants of humanity have spread from star to
star, they won't tell the children about the history of Ancient Earth until
they're old enough to bear it; and when they learn they'll weep to hear
that such a thing as Death had ever once existed!"

-Eliezer Yudkowsky

"Trust me" means "I love you" http://project-apollo.net/mos/mos114.html

"This isn't even my final form!"

-Tim Hausler

jack-trikeapps pushed a commit that referenced this pull request Mar 25, 2014
Merge pull request #94 from PotatoDumplings/interesting_comments
9915ca7 
Descendant karma attribute no longer disappears off python objects.
@jack-trikeapps jack-trikeapps merged commit 9915ca7 into bellroy:master Mar 25, 2014
@jack-trikeapps
Copy link
Contributor

I've merged this and will fix the whitespace myself. Hopefully I'll have time to do a full manual test on a staging server later today.

jack-trikeapps pushed a commit that referenced this pull request May 8, 2014
@endgame
Revert "Merge pull request #94 from PotatoDumplings/interesting_comme…
f297958 
…nts"

This reverts commit 9915ca7, reversing
changes made to 11fbb2f.

Conflicts:
	r2/r2/lib/db/tdb_sql.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@LucasSloan @jack-trikeapps