briot / geneapro

Genealogical software based on the GenTech data model
http://briot.github.com/geneapro/
GNU General Public License v2.0
33 stars 8 forks source link

Django 2 transition #52

Closed changeling closed 5 years ago

changeling commented 5 years ago

Added on_delete to ForeignKey definitions in models. Using PROTECT at the moment, though it might be worth considering which on_delete is best in each instance.

Removed version dependency from pip ... django ... in setup.sh.

changeling commented 5 years ago

From my limited testing, this seems to be the only barrier to working with django 2.x.

If there are concerns about further testing, perhaps it would be worth creating a django_2_transition branch and I could re-target this PR to that branch.

briot commented 5 years ago

I am ready to merge this one, but there are some conflicts because some of my own changes on HEAD also appear in the modified files, and have been modified since. I think with github you need to do pull requests from a branch (which I find unfortunate, has been a problem for me in other repositories)... Sorry for the inconvenience

changeling commented 5 years ago

This should be clean. I went ahead and reset the branch to your master, then redid the commits.

Oh, on a side note, one thing that may be useful with the more recent Django versions is the index-definition in a model's Meta (makemigrations automatically creates the necessary migration file), as in:

    class Meta:
        """Meta data for the model"""
        ordering = ("date_sort",)
        db_table = "place"
        indexes = [
                models.Index(fields=['id', 'date', 'date_sort', 'parent_place', 'name'], name='place_list-all_idx'),
        ]

I went through the following sequence:

All Places Clicked:
---
Log Entries:
2019-03-14 23:34:56,590 get geneaprove.views.places.PlaceList.get(<JSONViewParams: {}>)
2019-03-14 23:34:56,673 (0.080) SELECT "place"."id", "place"."date", "place"."date_sort", "place"."parent_place_id", "place"."name" FROM "place" ORDER BY "place"."date_sort" ASC; args=()
--
Query:
SELECT
    "place"."id",
    "place"."date",
    "place"."date_sort",
    "place"."parent_place",
    "place"."name"
FROM
    "place"
ORDER BY
    "place"."date_sort"
    ASC;
-
Django Model Meta-defined Index:
    class Meta:
        """Meta data for the model"""
        ordering = ("date_sort",)
        db_table = "place"
        indexes = [
                models.Index(fields=['id', 'date', 'date_sort', 'parent_place', 'name'], name='place_list-all_idx'),
        ]

Django-created Migration SQL:
CREATE INDEX "place_list-all_idx" ON "place" ("id", "date", "date_sort", "parent_place_id", "name");
---
briot commented 5 years ago

Indexes will definitely be useful. They will need careful consideration, of course...

changeling commented 5 years ago

Absolutely. I played with that a couple of others simply to experiment with the process and the migration path. But definitely worth consideration.