Skip to content

WIP: Add index support to Meta#1

Closed
akki wants to merge 86 commits into
gsoc-indexesfrom
gsoc-indexes-2
Closed

WIP: Add index support to Meta#1
akki wants to merge 86 commits into
gsoc-indexesfrom
gsoc-indexes-2

Conversation

@akki
Copy link
Copy Markdown
Owner

@akki akki commented Jun 19, 2016

No description provided.

@akki akki force-pushed the gsoc-indexes-2 branch from 9d47e1e to 92a7aef Compare June 28, 2016 16:55
@akki akki force-pushed the gsoc-indexes-2 branch from 2ff2a42 to efda4bb Compare June 29, 2016 17:17
Comment thread django/db/migrations/state.py Outdated
Copy link
Copy Markdown
Owner Author

@akki akki Jun 29, 2016

Choose a reason for hiding this comment

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

@MarkusH - Does this validation seem good to you about what we had discussed. The exact behavior we want to check is if index._name is set but I don't think using the private variable _name over here is a good idea. Any thoughts ?

Also for this validation we don't need to explicitly set the name of the index in the from_model method like here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would do the check in AddIndex() I think.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I agree that this check is required in AddIndex (and hence I have done that in my recent commit related to migrations) but I would like to know your thoughts on keeping a sanity-check here, because in the autodetector there is no use of a ModelState object with an index which doesn't have a name AKAIK (name is used in Index.__eq__). There might be other use cases of ModelState objects other than in autodetector that I am unaware of, in which case we might not want to keep this check.

@akki akki force-pushed the gsoc-indexes-2 branch from 3b1eecc to 7eff0f1 Compare June 30, 2016 04:49
Comment thread django/db/migrations/autodetector.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you probably want to handle this afer the generate_altered_index_together() method.

@akki akki force-pushed the gsoc-indexes-2 branch 2 times, most recently from 5be720b to 43bfacf Compare June 30, 2016 14:40
@akki akki force-pushed the gsoc-indexes-2 branch from a7a1342 to daf9239 Compare July 14, 2016 12:55
@akki akki force-pushed the gsoc-indexes-2 branch from daf9239 to aa44813 Compare July 14, 2016 17:20
willhardy and others added 3 commits July 14, 2016 13:34
Fixed a regression in 625b8e9:
improper short-circuiting could lead to a KeyError when threads
concurrently call RegexURLResolver._populate().
@akki akki force-pushed the gsoc-indexes-2 branch from aa44813 to 668ba24 Compare July 15, 2016 05:52
@akki akki force-pushed the gsoc-indexes-2 branch from 668ba24 to 7d7a26e Compare July 16, 2016 06:10
@akki akki force-pushed the gsoc-indexes-2 branch from 7d7a26e to 11c8997 Compare July 17, 2016 15:35
@akki akki force-pushed the gsoc-indexes-2 branch from 11c8997 to 8027608 Compare July 18, 2016 15:34
akki added 5 commits July 18, 2016 21:09
This will fully remove the dependency of the Index class
on it's 'model' attribute

Thanks timgraham for suggestion
Thanks timgraham for review and MarkusH for advice
Otherwise indexes made using class based indexes get dropped whenever
_remake_table is called.
@akki akki force-pushed the gsoc-indexes-2 branch from 8027608 to 2496a55 Compare July 18, 2016 15:45
@akki akki closed this Jul 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.