Skip to content

Conversation

@dAdil
Copy link

@dAdil dAdil commented Apr 28, 2024

Here, I add a few quality-of-life improvements to make the dev experience as smooth as possible:

  • Install the psql CLI into the container.
  • Install python requirements as part of container setup.
  • Set port forwarding the recommended way (oops, I had that misconfigured before).
  • Added a debugger configuration for vscode.
  • Added a migration script to create the DB and auto-apply the relevant schema updates on launch.

All included, a new developer should now just have to open the repository in a dev container enabled vscode and hit run!

@SamP20 I'm not sure how the migration script would fit in with your DB dump + sample data? I presume we could start with an empty DB, run your SQL script, set the "last_migrated.txt" to migration/19_address_not_null and debug from there?

migrate.sh Outdated
psql -h localhost -U postgres -d hackspace -f "$file"
echo "$file" > last_migrated.txt
fi
done
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This started life as a 1-liner and then grew. It should have probably become a python script as soon as I added statefulness via last_migrated.txt, but here we are :( If you feel strongly about it let me know and I'll convert it across.

@SamP20
Copy link
Member

SamP20 commented Apr 28, 2024

@SamP20 I'm not sure how the migration script would fit in with your DB dump + sample data? I presume we could start with an empty DB, run your SQL script, set the "last_migrated.txt" to migration/19_address_not_null and debug from there?

I'm wondering whether we should start using something like https://flask-migrate.readthedocs.io/en/latest/index.html? It stores the migration version in the database itself, ensuring the version always stays in sync. The easiest option would probably be to condense all our existing scripts into a single migration, then continue from there

I'm happy to use this script in the meantime though if it makes things a bit easier for you.

@SamP20 SamP20 deleted the branch bristolhackspace:wip/samp/sample-data April 29, 2024 10:18
@SamP20 SamP20 closed this Apr 29, 2024
@SamP20 SamP20 reopened this Apr 29, 2024
@dAdil
Copy link
Author

dAdil commented Apr 29, 2024

Ooh, I like this idea - I haven't really worked in this stack before which is why I jumped at automating sql via bash. It probably makes sense for me to keep this PR open and move across to that over the next week or 2 rather than uproot everyone's dev process only to do it again in a few weeks time.

Sorry this isn't in the thread, I can't seem to reply to comments - only create new ones...

dAdil and others added 3 commits May 2, 2024 19:46
…ntainer app and db images to ensure compatible versions of postgres tooling, re-dump sample_dataset to contain schema version OOTB
@dAdil
Copy link
Author

dAdil commented May 5, 2024

Sorry for the delay! I had some time to work on this earlier this week, but got lost in a dependency versioning mess. I've pinned certain versions of the python dev container and the postgres images so that we can't mix up pgsql vs CLI versions when doing pg_dumps etc in future.

This PR is now a breaking change. If you have a DB already, you will need to do the following:

  1. Ensure all of the the previous migrations were applied.
  2. Checkout the latest change.
  3. Gather the new python dependences (via pip, virtual environment, rebuilding dev container etc).
  4. Run flask --app hackspace_mgmt db stamp head to apply the initial schema version change.
  5. Start the app as usual.

I'm happy to be on hand during the real upgrade if we are concerned, but taking a backup and using that as a roll-back strategy is probably the safest bet.

@SamP20
Copy link
Member

SamP20 commented May 7, 2024

This is amazing work, thank you! I'll try and find some time to give it a proper review, especially the new migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants