Populate Course and Gened tables with new data

[nsandler1]

Depends on #89

creates a migration script to set up the database by updating all the courses and geneds following the schema changes in #89.

[Liam-DeVoe]

not sure how I feel about this existing as a migration. This effectively freezes umdio as a dependency in the migration chain, even years down the line when it may have moved to a new domain or changed schema. Is there any reason this has to exist as opposed to telling developers to populate geneds manually? The field is already nullable and I don't think anything on the site will break if it's not populated.

[Liam-DeVoe]

To add to that, migrations are really meant for transforming existing data or schemas, not populating with new data.

[nsandler1]

To add to that, migrations are really meant for transforming existing data or schemas, not populating with new data.

And that's exactly what this script does. It makes sure that our gened data is consistent with umdio's data. I made this so neither one of us would have to run #91 for every single semester we have a record of.

not sure how I feel about this existing as a migration. This effectively freezes umdio as a dependency in the migration chain, even years down the line when it may have moved to a new domain or changed schema. Is there any reason this has to exist as opposed to telling developers to populate geneds manually? The field is already nullable and I don't think anything on the site will break if it's not populated.

Nothing on the site would break but there are many holes in our gened data that go all the way back to the beginning so this is just an all-in-one way to correct things.

[Liam-DeVoe]

I made this so neither one of us would have to run #91 for every single semester we have a record of.

why would we need to run it for multiple semesters? we should only have to run the gened script once, to get the latest gened data for all courses.

[Liam-DeVoe]

And that's exactly what this script does. It makes sure that our gened data is consistent with umdio's data.

it's not simply transforming existing data in the db, it's pulling new data that didn't exist in the db before. We didn't store the gened json of the courses before. But more to the point, migrations should only ever depend on data already in the db. Relying on an external api in a migration script is a recipe for disaster. This will break at some indeterminant point in the future and will cause a headache when it does.

Future developers will already have to run the gened script again to get the latest geneds, so I don't see the issue with requiring that from the get-go (and it's not even a requirement if they don't touch anything gened related).

[nsandler1]

I made this so neither one of us would have to run #91 for every single semester we have a record of.

why would we need to run it for multiple semesters? we should only have to run the gened script once, to get the latest gened data for all courses.

outside of this one time we won't. We need to update all the courses, not just the courses for a particular semester. This doesn't need to be in the form of a migration, that was just the first thing that came to mind for "single use scripts". I can run the script locally when the time comes and just import my DB to prod if that works better. Specifically, I would import prod to my local db, run the script, then import my db back into prod.

[Liam-DeVoe]

I didn't look at the script closely enough - I assumed this was iterating over all the courses in the database and retrieving geneds per course, rather than only iterating over courses from umdio. Is there a reason the script doesn't use the former? That's what I was expecting and would update all courses at once, only requiring running the script once.

I would also expect the management command to iterate over courses in the db, fwiw.

[nsandler1]

I went back and fourth with this actually. I settled on doing things this way for two reasons: 1) Only a subset of our courses are actually offered during any particular semester so I didn't think it made sense to go through all our courses every time we ran the script. 2) We have courses that umdio doesn't have so this method avoids another check.

[Liam-DeVoe]

I don't think it makes sense to restrict to a particular semester. If we forget to update geneds for semester A, then update it later in semester B, we have to remember to also run the command for all semesters we missed before semester B. It costs us nothing to run for all courses and ensure full db integrity every time, except the script taking longer, which is a nonissue as we only have to run this once a semester in prod and it can be run in the background (or as a cron).

[nsandler1]

Just to clarify, are you saying we should just have one script that goes through all our courses and updates everything? If so, then I'm ok with that but I still think there's value in having an optional argument to specify the semester.

[Liam-DeVoe]

if by "goes through all our courses and updates everything" you mean "goes through all our courses and updates their geneds", then yes, that's what I'm suggesting the gened script in #91 becomes. If by "everything" you mean "geneds and also recency and also..." then I do think there's value in that, but would suggest the following structure:

management/commands
  updategeneds.py
  updaterecency.py
  updateeverything.py

where updateeverything.py does nothing more than call out to updategeneds.py and updaterecency.py.

An optional semester arg is fine by me.

[nsandler1]

yes I only meant updating the geneds but I can make a separate pr for updateeverything.py. So then I guess we can close this pr and I'll just change #91 into what we've discussed. That is, a script that goes through all our courses and updates their geneds based on the data available on umdio. The script will take an optional semester argument but if no semseter is specified the script will update for the most recent semester available on umdio.

[Liam-DeVoe]

yep, that sounds good to me.

[Liam-DeVoe]

going to close this pr as discussed. Will live on in #91