Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refine migration script #1617

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

Refine migration script #1617

wants to merge 13 commits into from

Conversation

Mythicaeda
Copy link
Contributor

@Mythicaeda Mythicaeda commented Dec 9, 2024 •
edited
Loading

Description

Adds a tracked view to Hasura showing the integer contents of the migrations.schema_migrations table. This allows Aerie Admins on a venue to view which migrations have been applied (was formerly locked behind Hasura Admins/Postgres Admins).

Heavily refactors the Aerie DB Migration Script:

  • Adds the 'status' subcommand to view the current status of migrations without applying them
  • Moves the migration logic into the 'migrate' subcommand
  • Adds a Hasura class for communicating via the CLI or API. This ensures that all necessary flags are always passed to the CLI. Necessary flags include: the Hasura project root, the Hasura endpoint and admin secret, the location of the .env to load values from, the database to apply migrations to
  • Loads in endpoint and admin secret information the same way that the Hasura CLI does (flags, then envvars, then .env, then config.yaml)
  • Gets the current database migration status by running a run_sql query against the Hasura venue, removing the need to provide database connection information
  • Fixes the bug where Hasura metadata would be reloaded but not applied, forcing the user to restart Hasura. The new metadata is now applied before it's reloaded

Verification

Manually tested

Documentation

Added doc comments and type hints throughout the script.

The website docs were edited to reflect the changes to the command line arguments and introduction of subcommands.

Future work

  1. Refining the output when a migration fails to run. Currently the blob JSON is printed and it's hard to parse even if you know what you're looking for.
  2. Perhaps adding an "until" flag to the bulk migration (as in, "apply[revert] migrations until you reach this one"). Not sure if there's a pressing use case for this currently.

@Mythicaeda Mythicaeda added refactor A code change that neither fixes a bug nor adds a feature fix A bug fix labels Dec 9, 2024
@Mythicaeda Mythicaeda self-assigned this Dec 9, 2024
@Mythicaeda Mythicaeda requested a review from a team as a code owner December 9, 2024 23:48
@Mythicaeda
Add admin-only view to display applied migrations
2f04aff 
This view allows Aerie admin users to read migration information about their venue (information was formerly locked to Hasura/Postgres Admins)
@Mythicaeda
Extract repeated exit code into exit_with_error
5f71792
@Mythicaeda
Extract migration steps inside DB_Migration initializer
050a6e7 
Additionally, record where the migration steps are located and update step-by-step mode to reference this location
@Mythicaeda
Extract creating args parser to a method
56f5e00
@Mythicaeda
Update Configuration Loading
92d9f2f 
Hasura CLI loads first from arguments passed to the command, then from environment variables, then from a passed .env, then finally from a config file. This change updates how the script determines its connection credentials to match how HasuraCLI works.
@Mythicaeda
Use Hasura endpoint to fetch current migration information
2d2221d 
Changes the script to use a `run_sql` request to get the current migration, rather than a direct connection to a database.

This removes the ability for the user to specify one database in the 'netloc' argument and another in the Hasura configuration
@Mythicaeda
Create CLI Wrapper Class
aef4d2a 
Hasura replaces the os.system and subprocess.getoutput calls. This was done as there are three flags that need to be provided to every call to the hasura cli:

- skip-update-check: cleans up the output by skipping the automatic update check
- project: directory where the cli command is executed
- envfile: env file to load envvars from. Set to '.env' in the directory the CLI is run from to avoid the CLI and the Migration script loading separate envfiles
@Mythicaeda
Print the Hasura Endpoint that the migration script is migrating
229f1c3
@Mythicaeda
Create 'status' subfunction
bdeaac4 
- Extracts initializing a Hasura object from a Namespace into a function
- moves migration logic into 'migrate' subfunction
@Mythicaeda
Reorder functions
eafb1c2
@Mythicaeda
Update doc comments
e912936
@Mythicaeda
Update pgcmp workflow
174f6b3
@Mythicaeda Mythicaeda force-pushed the fix/refine-migration-script branch from 2c68885 to 174f6b3 Compare December 10, 2024 15:17
@Mythicaeda
Fix Hasura failing to reload metadata
0fa71a3 
The migration script now runs "hasura metadata apply" before "hasura metadata reload".
  - As metadata reloading takes noticeably longer, in step-by-step mode it now only runs when exiting, rather than between each step.
@Mythicaeda Mythicaeda mentioned this pull request Dec 11, 2024
Open
@Mythicaeda Mythicaeda added the breaking change A change that will require updating downstream code label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dandelany dandelany Awaiting requested review from dandelany

@mattdailis mattdailis Awaiting requested review from mattdailis

At least 1 approving review is required to merge this pull request.

Assignees

@Mythicaeda Mythicaeda

Labels
breaking change A change that will require updating downstream code fix A bug fix refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Milestone
No milestone
1 participant
@Mythicaeda