Redesign rbdemo fixture loading for safety and better maintenance.

Review Request #12403 — Created June 23, 2022 and submitted

Information

rb-extension-pack
master

Reviewers

rbdemo has always been a bit hard to maintain. We relied on Django's
fixture support to dump out state and load it in, and this doesn't
really work well when moving between versions of Django or Review Board.
It's also just really hard to maintain the data we want to put up.

This is a complete redesign of how we deal with state for rbdemo.

First, safety. We no longer use Django's management commands to reset
database state. These run in, and commit, their own transactions,
meaning that if we fail to load state after we reset, we just end up
with an empty database.

We now handle the flushing/resetting process ourselves, but more
selectively. Tables essential to running the server (site, siteconfig,
evolutions, migrations, etc.) are kept intact. This also makes it much
easier to set up a mock demo server for development.

Everything is now run in a single transaction. If anything goes wrong,
we shouldn't lose anything already in the database. This makes periodic
resetting safe.

The idea of how we load data has also been entirely redone.

We no longer use Django's fixtures support. Instead, we load a YAML file
(or files) that contains basic structured data needed to populate a demo
server. This is easier to maintain, as we can just edit some content in
the files and re-import into a demo server.

Since we're in full control of the loading process now, and are less
tied to exact database state, we should have a lot fewer problems
updating this as we upgrade Review Board.

Tested reloading demo data with errors. The server no longer had to be
reset.

Built a YAML document containing all the content from demo.reviewboard.org
and have successfully been able to import it, edit it, and re-import it
numerous times. The content on the server fully matched expectations.

Summary ID
Redesign rbdemo fixture loading for safety and better maintenance.
rbdemo has always been a bit hard to maintain. We relied on Django's fixture support to dump out state and load it in, and this doesn't really work well when moving between versions of Django or Review Board. It's also just really hard to maintain the data we want to put up. This is a complete redesign of how we deal with state for rbdemo. First, safety. We no longer use Django's management commands to reset database state. These run in, and commit, their own transactions, meaning that if we fail to load state after we reset, we just end up with an empty database. We now handle the flushing/resetting process ourselves, but more selectively. Tables essential to running the server (site, siteconfig, evolutions, migrations, etc.) are kept intact. This also makes it much easier to set up a mock demo server for development. Everything is now run in a single transaction. If anything goes wrong, we shouldn't lose anything already in the database. This makes periodic resetting safe. The idea of how we load data has also been entirely redone. We no longer use Django's fixtures support. Instead, we load a YAML file (or files) that contains basic structured data needed to populate a demo server. This is easier to maintain, as we can just edit some content in the files and re-import into a demo server. Since we're in full control of the loading process now, and are less tied to exact database state, we should have a lot fewer problems updating this as we upgrade Review Board. The old `dump-demo-data` command is now gone, as the YAML data is now a source document and not just a database backup.
3ed7d2bf8a47f263f640f6ed6b691fbf1728be5e
Description From Last Updated

What happens if a _load function is missing some required fields for the object it's trying to load? For example …

maubinmaubin

"with" instead of "which"

maubinmaubin

"the" instead of "teh"

maubinmaubin
maubin
  1. 
      
  2. Show all issues

    What happens if a _load function is missing some required fields for the object it's trying to load? For example if a User object is missing a username. Should we check for this and log an error saying what's wrong/missing in the YAML file? Also might be a good idea to include an example YAML file somewhere.

    1. I'm taking the lazy approach for now, since this is really for our own use, and there's already an established set of data files (which will be coming up shortly on RBCommons).

      I didn't want to spend too much time on making this more generally-usable (there's lots of data we don't currently support, for example), since there's so much else to do. People are more likely to get use out of Power Pack's Import/Export feature than this, which does do checks like these.

    2. Ah got it.

  3. Show all issues

    "with" instead of "which"

  4. Show all issues

    "the" instead of "teh"

    1. Also "table sand" -> "tables and"

    2. Downside of writing comments and docs at 4:30AM :(

  5. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (b705b48)
Loading...