Loosen requirements

[efischer19]

@doctoryes is this a thing we do, organizationally speaking? I know it adds a bit of risk (what if the package maintainer does something evil with a minor version release?), but if everyone uses semver properly it should allow for easier upgrades. I need to loosen dogapi in particular in order to do something similar for ora's requirements

(in the process of a setup.py install for edx-ora2)

Processing dependencies for ora2==1.4.8
error: dogapi 1.11.1 is installed but dogapi==1.2.1 is required by set(['edx-submissions'])

[doctoryes]

These djangoapp modules should have their requirements pinned as loosely as possible - basically only guarding against known incompatibilities or future, likely incompatibilities released under a larger semver number. It should be the encompassing Django project that decides on the version within that loose range.

[doctoryes]

Looks like UUIDField is used from django-extensions....

[efischer19]

was used, there's a better way

[efischer19]

@doctoryes, this is ready for review now. I've loosened all the requirements versions, eliminated django-extensions (as it's no longer needed), and touched up a bit of string-vs-UUID logic related to that removal to keep tests happy.

[doctoryes]

LGTM - as long as migration implications are figured out.

[doctoryes]

What's the deployment implications of this migration? How big is the submissions_submission table on production?

[efischer19]

This is a good question - the only change is a semantic one around the uuid field (it has no practical effect on normal usage), but the output of manage.py sqlmigrate is not something I'd expect:

BEGIN;
ALTER TABLE `submissions_submission` ALTER COLUMN `uuid` SET DEFAULT '240af22377d44f82817ee79b1a5fd61d';
ALTER TABLE `submissions_submission` ALTER COLUMN `uuid` DROP DEFAULT;

COMMIT;

The size of the table is 1.7M rows in prod. However, I would argue that the above is effectively a no-op, and we ought to just skip applying the migration entirely.

@jibsheet, any suggestions here? I'm attempting to remove the django_extensions requirement on this project, and it's used in a model field. That change necessitated a migration, which when poked seems to do nothing, effectively. Can we declare this "safe to skip" and include a --fake in the GoCD rollout of this code change? Is that even possible?

If this turns out to be too complex, I'm fine with reverting the django_extensions removal out.

[jibsheet]

Last time I ran across this, it's a django stupidity but MySQL Innodb treats it as a metadata change: https://github.com/edx/edx-platform/pull/14830#discussion_r110185023

You can probably make a million row table on a sandbox to test, but when I did something similar for Christina, it was fine. I'll mention that the behavior might differ on nullable fields, but I'd be shocked if a uuid field was NULLable.

[efischer19]

Oh awesome, thanks for the links @jibsheet. I'll test this out on a sandbox tomorrow and get back to you.

[efischer19]

Running against a sandbox with 200K rows on the table in question:

edxapp@ip-192-168-0-15:~/edx-platform$ time ./manage.py lms --settings=aws migrate submissions 0004
<lots of django warnings>
2017-08-02 09:56:21,582 INFO 12833 [dd.dogapi] dog_stats_api.py:66 - Initializing dog api to use statsd: localhost, 8125
Operations to perform:
  Target specific migration: 0004_remove_django_extensions, from submissions
Running migrations:
  Rendering model states... DONE
  Applying submissions.0004_remove_django_extensions... OK

real	0m6.412s
user	0m5.656s
sys	0m0.676s

From my perspective, the Applying submissions.0004_remove_django_extensions portion was nearly instantaneous, the 5 seconds were spent in django warmups and model state rendering.

[jibsheet]

Yep, that sounds reasonable

[doctoryes]

Good to see dependencies removed!

[efischer19]

Per @jibsheet and I's offline discussion, I'm going to push this out to prod separate of openedx/edx-ora2#1023