Extend make up to automatically initialise the database

[justinclift]

What type of PR is this?

• Other

Description

This PR updates make up to automatically initialise the database if that hasn't already been done.

It does this by (very quickly) checking if the organization table is present, then running make create_database if not.

This is a re-application of #6855, that was caught up in the large revert a while back.

How is this tested?

• Manually

This shouldn't affect anything with the running code, and seems to work fine (!) in my local development environment.

Related Tickets & Documents

#6855

[gaecoli]

LGTM!

[justinclift]

Awesome, thanks @gaecoli. 😄

[zachliu]

on a similar line of thought, maybe we should also add /app/manage.py db upgrade at scheduler, worker, server start-up?

for example

scheduler() {
  echo "Upgrading database..."
  /app/manage.py db upgrade
  echo "Starting RQ scheduler..."

  exec /app/manage.py rq scheduler
}

no one joined my discussion topic #7005 so i ask here 😁

[justinclift]

Oh, I didn't notice your discussion about it.

So, you're pretty much suggesting automatic database upgrades? I'd be in favour of that, and have previously stated (maybe in Discord?) that's something I want to get happening.

Did the Mozilla approach of having it run as part of the server/worker/scheduler entrypoint cause any issues that should be looked into?

[zachliu]

yeah, automatic database upgrades 👍

Did the Mozilla approach of having it run as part of the server/worker/scheduler entrypoint cause any issues that should be looked into?

i have been on the mozilla fork for a while and when i switched back here i immediately added the /app/manage.py db upgrade to my implementation. haven't noticed any issues.

[justinclift]

Cool. Want to throw together a suitable PR or cherry pick Mozilla's one across? 😄

[zachliu]

PR coming right up!💻

[zachliu]

PR is up. not sure what's going on with cypress 😿

[justinclift]

Looking over #7166 it looks fine to me apart from possibly needing a blank line removed (and that's if I'm being picky).

I'm not (yet) understanding why #7167 is needed as well though. That's introducing a sleep, but shouldn't that not be needed for #7166? It kind of seems like #7166 should be enough by itself, though I could just need more coffee... 😄

[zachliu]

@justinclift are you referring to this line?

execSync("docker compose -p cypress up -d", { stdio: "inherit" });

if this execSync expects to "block" until everything is done, then i don't think it works because of the -d

i did some experiments on my own fork. looks like adding db upgrade in #7166 would make the yarn cypress start -- --skip-db-seed run longer, getting in the way of the next docker compose run cypress yarn cypress db-seed so i added #7167. also, there has been a sleep 10 in the same workflow

redash/.github/workflows/ci.yml

Lines 50 to 51 in 38dc31a

	docker compose up -d
	sleep 10

i guess it was added there for a similar reason