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

Adding a wait for sql strategy #122

Merged
merged 3 commits into from
Feb 3, 2020

Conversation

erdemtoraman
Copy link
Contributor

#121

A simplified sql db waiting strategy without adding any new dependency to the library. An example usage for a postgres container:

	var env = map[string]string{
		"POSTGRES_PASSWORD": "password",
		"POSTGRES_USER":     "postgres",
		"POSTGRES_DB":       "service",
	}

	var port = "5432/tcp"

	dbURL := func(port nat.Port) string {
		return fmt.Sprintf("postgres://postgres:password@localhost:%s/service?sslmode=disable", port.Port())
	}

	req := testcontainers.GenericContainerRequest{
		ContainerRequest: testcontainers.ContainerRequest{
			Image:        "postgres:latest",
			ExposedPorts: []string{port},
			Cmd:          []string{"postgres", "-c", "fsync=off"},
			Env:          env,
			WaitingFor:   wait.ForSQL(nat.Port(port), "postgres", dbURL).Timeout(time.Second*5),
		},
		Started: true,
	}
	container, err := testcontainers.GenericContainer(context.Background(), req)

	if err != nil {
		log.Fatal("start ", err)
	}
	mappedPort, err := container.MappedPort(context.Background(), nat.Port(port))
	if err != nil {
		log.Fatal(err)
	}
	defer container.Terminate(context.Background())

	log.Println("postgres container ready and running at port: ",mappedPort)
	time.Sleep(time.Minute)
	time.Sleep(time.Minute)

)

//ForSQL constructs a new waitForSql strategy for the given driver
func ForSQL(port nat.Port, driver string, url func(nat.Port) string) *waitForSql {
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about renaming this to ForDB and waitForDB? At the end of the day, we are waiting for the database. Using SQL for it seems an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good point but I assumed it'd be more consistent to point out "sql" because it can only handle sql based databases, if we just call it forDB the client of this function might not immediately understand it'd not work for say mongodb.

Copy link
Member

Choose a reason for hiding this comment

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

I agree in the sense using SQL distinguish from any other storage type. My only concern was related to my mind directly connecting waitForSQL to wait for the execution of an SQL sentence... which on the other hand is true as we use SELECT 1.

So I'm good with this one :)

@ClaytonNorthey92
Copy link
Contributor

could we add tests for this? maybe at least one for every SQL database type we support?

could we also add some documentation for this in the README?

@nikolayk812
Copy link

@gianarb what do you think about this PR?

@gianarb
Copy link
Member

gianarb commented Jan 30, 2020

I would like to get it merged! @ClaytonNorthey92 @mdelapenya thumbs up and I will click the button!

@gianarb gianarb merged commit c0c9a5c into testcontainers:master Feb 3, 2020
@gianarb
Copy link
Member

gianarb commented Feb 3, 2020

Thanks a lot @erdemtoraman !!
I created an issue to get an Example func for this feature #142

If anybody is up for it

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.

5 participants