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

Mysql canned container #59

Open
wants to merge 1 commit into
base: master
from

Conversation

@gianarb
Copy link
Member

commented Apr 20, 2019

Reference #19

I bootstrapped a mysql canned container.

ctx := context.Background()
c, err := MySQLContainer(ctx, MySQLContainerRequest{
        RootPassword: "root",
        Database:     "hello",
})
if err != nil {
        t.Fatal(err.Error())
}
defer c.Container.Terminate(ctx)
sqlC, err := c.GetDriver("root", "root", "hello")
if err != nil {
        t.Fatal(err.Error())
}
_, err = sqlC.ExecContext(ctx, "CREATE TABLE example ( id integer, data varchar(32) )")
if err != nil {
        t.Fatal(err.Error())
}

Signed-off-by: Gianluca Arbezzano gianarb92@gmail.com

Mysql canned container
Reference #19

I bootstrapped a mysql canned container.

```
ctx := context.Background()
c, err := MySQLContainer(ctx, MySQLContainerRequest{
        RootPassword: "root",
        Database:     "hello",
})
if err != nil {
        t.Fatal(err.Error())
}
defer c.Container.Terminate(ctx)
sqlC, err := c.GetDriver("root", "root", "hello")
if err != nil {
        t.Fatal(err.Error())
}
_, err = sqlC.ExecContext(ctx, "CREATE TABLE example ( id integer, data varchar(32) )")
if err != nil {
        t.Fatal(err.Error())
}
```

Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
@gianarb

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

@mraerino @alde what do you think?

@mraerino
Copy link
Contributor

left a comment

some comments, but I like the general approach

ctx context.Context
}

func (c *mysqlContainer) GetDriver(username, password, database string) (*sql.DB, error) {

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

Consider taking the parameters from the request and fallbacking to the defaults of the image

return nil, err
}

req.Image = "mysql:8.0"

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

Consider making the version configurable from the request and having a default in a package constant.

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

Maybe also make the image configurable so somebody can choose a custom image that is compatible with the options of the default image

req.Image = "mysql:8.0"
req.ExposedPorts = []string{"3306/tcp"}
req.Env = map[string]string{}
req.Started = true

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

All these options basicly overwrite options that may be set by a user. Try checking if this is empty in the ContainerRequest

req.Env["MYSQL_DATABASE"] = req.Database
}

req.WaitingFor = wait.ForLog("port: 3306 MySQL Community Server - GPL")

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

The java implementation actually tries connecting via a mysql client to verify that the database is ready.

Container: c,
}
mysqlC.req = req
mysqlC.ctx = ctx

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

This is not a common pattern since a context is usually valid for a single function call. Consider e.g. context.WithTimeout()

if err := c.Start(ctx); err != nil {
return mysqlC, errors.Wrap(err, "failed to start container")
}
}

This comment has been minimized.

Copy link
@mraerino

mraerino Apr 23, 2019

Contributor

is this functionality not implemented in the testcontainer-go core?

@alde

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Sorry, I've been away on vacation with limited internet access.

Love the direction, and I agree with most of @mraerino's points :)

@gianarb gianarb force-pushed the master branch from 8e48566 to b024306 Aug 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.