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

Add getting started docs and Postgres tutorial #258

Merged
merged 18 commits into from
Aug 13, 2019

Conversation

mknycha
Copy link
Contributor

@mknycha mknycha commented Aug 7, 2019

No description provided.

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

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

Thanks for taking a stab at this and opening the PR! This is a great start!

I think it'd be better to introduce concepts and best practices in the "Getting Started" doc and have DB specific examples in each DB.
It makes sense to rename TUTORIAL.md to GETTING_STARTED.md, move the postgres specific example/tutorial to databases/postgres/TUTORIAL.md, and create a table of contents for DB specific tutorials in GETTING_STARTED.md.

FAQ.md Outdated
@@ -68,3 +68,6 @@
Database-specific locking features are used by *some* database drivers to prevent multiple instances of migrate from running migrations at the same time
the same database at the same time. For example, the MySQL driver uses the `GET_LOCK` function, while the Postgres driver uses
the `pg_advisory_lock` function.

#### Do I need to create a table for tracking migration version used?
No, it is done automatically. This information will be kept in the table `schema_migrations` in the database that you have configured.
Copy link
Member

Choose a reason for hiding this comment

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

The actual schema migrations table name is set by the DB driver. Generally, it's schema_migrations, but that name is not enforced. I haven't audited the names used by all the DB drivers, so I don't know how accurate this statement is. To be safe and more resilient to future changes, let's not make such an authoritative statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

TUTORIAL.md Outdated
@@ -0,0 +1,100 @@
# Getting started
Before you start, make sure you understand the concept or database migration and rollback.
Copy link
Member

Choose a reason for hiding this comment

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

Rephrase to "make sure you understand the concept of forward/up and reverse/down database migrations"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed

TUTORIAL.md Outdated
# Getting started
Before you start, make sure you understand the concept or database migration and rollback.

Create new Go project and configure a database for it. Make sure that your database is supported [here](README.md#databases)
Copy link
Member

Choose a reason for hiding this comment

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

Migrate can be used to manage migrations with non-Go projects via the CLI, so creating a Go project isn't necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Is it worth adding it to FAQ?

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that migrate is primarily used with Go projects today, but that doesn't have to be the case. No one has asked the question yet, but we could add it to the FAQ 😄

e.g.
Can I use migrate with a non-Go project?
Yes, you can use the migrate CLI in a non-Go project, but there are probably other libraries/frameworks available that offer better test and deploy integrations in that language/framework.

Copy link
Contributor Author

@mknycha mknycha Aug 8, 2019

Choose a reason for hiding this comment

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

Let's assume I have asked it three times recently :D but honestly, I think it will not be harmful to add it there ;)

TUTORIAL.md Outdated

Create new Go project and configure a database for it. Make sure that your database is supported [here](README.md#databases)
For the purpose of this tutorial let's create PostgreSQL database called `example`.
Our user here is `postgres`, and host is `localhost`.
Copy link
Member

Choose a reason for hiding this comment

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

Also mention what the password is. I'd use a password that's distinct from the username for clarity

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

TUTORIAL.md Outdated
## Create migrations
Let's create table called `users`:
```
migrate create -ext sql -dir db/migrations create_users_table
Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend using the -seq option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it makes the example easier to understand but using -seq in a project used by many developers can go awry as they can push two migrations with the same sequential number. I feel like it's one of these "know what you're doing" options, while I wanted the tutorial to be accessible for someone who did not use database migrations before. What's your opinion here?

Copy link
Member

Choose a reason for hiding this comment

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

Detecting conflicting migrations is an unresolved issue. Using sequentially numbered vs timestamps doesn't fix the issue. e.g. when using timestamps, 2 different developers could create conflicting migrations and the developer who created created the migration second gets their PR merged before the first one.

Could you also mention this caveat (that conflicting migrations aren't detected) in our docs.

For more info, see: #179 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not thought about that. I will mention it in getting started docs.

TUTORIAL.md Outdated
```
migrate create -ext sql -dir db/migrations create_users_table
```
If there were no errors, we should have two files available under `db/migrations` folder. Note the `sql` extension that we provided.
Copy link
Member

Choose a reason for hiding this comment

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

Be explicit in the example and actually list the files that were created

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

TUTORIAL.md Outdated
If there were no errors, we should have two files available under `db/migrations` folder. Note the `sql` extension that we provided.
In the `.up.sql` file we are going to create table:
```
CREATE TABLE users(
Copy link
Member

Choose a reason for hiding this comment

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

Should also mention idempotency in migrations and use that in the examples. e.g. IF NOT EXISTS and IF EXISTS and wrapping your migration in a transaction if DDL transactions are supported by your DB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

TUTORIAL.md Outdated
(e.g. if you created a table in a migration but rollback did not delete it, you will encounter an error when running the migration again)
It's also worth checking your migrations in a separate, containerized environment. You can find some tools in the end of this tutorial.

**Hint:** Most probably you are going to use to the above commands often, it's worth putting them in a Makefile of your project.
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide more background on why you found wrapping the CLI usage in a Makefile useful?
I've been using the CLI directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In our project at work, we use it to call make migrate-up POSTGRESQL_DB=example_test steps=2 instead of migrate -database postgres://user:password@localhost:5432/example_test -path ./migrations/postgres up 2. It's just slightly shorter compared to putting database URL in .env file.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh, I normally have my Go apps use migrate as a library and call migrate.Migrate.Up() before serving traffic. Managing migrations out-of-app using the CLI makes sense too. e.g. there are many different dev and deploy processes
I wouldn't assume that all projects are using make to manage their development and/or deploys.

Copy link
Contributor Author

@mknycha mknycha Aug 9, 2019

Choose a reason for hiding this comment

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

Right, it may be too much environment-specific. I will remove this part

TUTORIAL.md Outdated
```
You can find details [here](README.md#use-in-your-go-project)

Just add the code to your app and you're ready to go!
Copy link
Member

Choose a reason for hiding this comment

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

Add a caveat about not all DBs supporting locking when running migrations, so if you run multiple instances of your app on different machines, you may encounter issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@dhui
Copy link
Member

dhui commented Aug 7, 2019

Addresses: #40

@mknycha
Copy link
Contributor Author

mknycha commented Aug 9, 2019

Just wanted to let know that I will reorganize files during the weekend.

@mknycha mknycha changed the title Add tutorial for getting started Add getting started docs and Postgres tutorial Aug 12, 2019
Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

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

Wow, this looks great! Only a few minor quibbles left.

```
Once you create your files, you should fill them.

**IMPORTANT:** In a project developed by more than one person there is a small probability of migrations incosistency - e.g. two developers can create conflicting migrations, and the developer that created his migration later gets it merged to the repository first.
Copy link
Member

Choose a reason for hiding this comment

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

Remove the word "small" since the chances increase with team size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Once you create your files, you should fill them.

**IMPORTANT:** In a project developed by more than one person there is a small probability of migrations incosistency - e.g. two developers can create conflicting migrations, and the developer that created his migration later gets it merged to the repository first.
Keep an eye on such cases (and be even more careful when cherry picking).
Copy link
Member

Choose a reason for hiding this comment

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

Remove reference to cherry picking since that doesn't increase the likelihood of an inconsistency occurring. I'd just note that developers and teams should keep any eye out for this issue, especially during code review. A note or link to this summary would be nice too. Namely it'd be good to get more input on the what's the best solution for this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. What I meant here is that someone could cherry pick changes including migration dependent on some earlier migrations.

@dhui
Copy link
Member

dhui commented Aug 13, 2019

Thanks for improving our docs! This should make it easier for new users to get started and help clarify some points/usage for existing users.

@dhui dhui merged commit b071731 into golang-migrate:master Aug 13, 2019
@dhui dhui mentioned this pull request Aug 13, 2019
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