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.

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

[Persistence] Implement proper SQL migration strategy (i.e. move SQL table schemas to .sql files) #77

Open
andrewnguyen22 opened this issue Apr 23, 2022 · 2 comments
Assignees
Labels
core Core infrastructure - protocol related persistence Persistence specific changes

Comments

@andrewnguyen22
Copy link
Contributor

@Olshansk
For this file (persistence/schema/account.go) and all the others, I wanted to add a point about migrations & schemas that I feel pretty strongly about before moving forward.

I looked at different SQL migration tools in Golang and though I haven't picked a specific one, golang-migrate/migrate seems to be the most popular one. However, instead of defining our tables in strings withing .go file, we should follow the best practices for how to manage migrations (note that creation is a migration in itself).

Source: golang-migrate/migrate
tl;dr

We store these in .sql files
Each change will have a .up.sql and a .down.sql file
This will enable:
Cleaner & easier to read code (separation of go and SQL)
Going back & forth during development
Easy to track changes / migrations when we actually do them on chain.

@derrandz
Copy link
Contributor

derrandz commented Jun 6, 2022

Adding to this a comment I put as a review for #73

We can make use of the following tool: dbmate. This tool will basically run in a container alongside the database, and will receive the database URL as an environment variable.

This will allow us to create very simple migrations in SQL, by simply using text directives:

-- migrate:up

-- migrate:down

So an example of a migration would be:

-- migrate:up
create table users (
  id integer,
  name varchar(255),
  email varchar(255) not null
);

-- migrate:down

This migration file will be generated using a command (dbmate new create_users_table) and we will fill it however we see fit.

Then, running the migrations up or down becomes a matter of calling a make command that will send the instruction to the tool in the container, which will subsequently run it against the database, the command would be something like:

migration-up:
	docker exec -v $(PWD)/migrations:/db migrater dbmate up

migration-down:
	docker exec -v $(PWD)/migrations:/db migrater dbmate rollback

And to generate new timestamped migrations:

generate_migration:
   docker exec -v $(PWD)/migrations:/db migrater dbmate new action_table

@derrandz derrandz changed the title Move sql table schemas to .sql files with proper migration strategy [Persistence]: Move SQL table schemas to .sql files with proper migration strategy Jun 6, 2022
@Olshansk Olshansk changed the title [Persistence]: Move SQL table schemas to .sql files with proper migration strategy [Persistence] Move SQL table schemas to .sql files with proper migration strategy Jun 6, 2022
@Olshansk Olshansk added this to the V1 Persistence Foundation milestone Jul 6, 2022
@Olshansk Olshansk assigned okdas and unassigned andrewnguyen22 Jul 16, 2022
@Olshansk Olshansk added the core Core infrastructure - protocol related label Jul 26, 2022
@Olshansk Olshansk added the community Open to or owned by a non-core team member label Aug 3, 2022
@Olshansk Olshansk changed the title [Persistence] Move SQL table schemas to .sql files with proper migration strategy [Persistence] Implement proper SQL migration strategy (i.e. move SQL table schemas to .sql files) Aug 3, 2022
@Olshansk Olshansk added core starter task Good for newcomers, but aimed at core team members though still open for everyone and removed starter task labels Nov 9, 2022
@jessicadaugherty jessicadaugherty removed community Open to or owned by a non-core team member core starter task Good for newcomers, but aimed at core team members though still open for everyone labels Nov 28, 2022
@Olshansk
Copy link
Member

@Gustavobelfort Not an immediate priority but this is something we will definitely need and I have a sense you might have some insights on it. It'll cross the boundary of the persistence module implementation and dev tooling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core infrastructure - protocol related persistence Persistence specific changes
Projects
Status: Backlog
Development

No branches or pull requests

7 participants