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

Added a charset field to dsn. #11

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Conversation

mikespook
Copy link

Hi, ziutek.

mymysql is a really amazing package, I would use it in my own project.

Based on the discussion in the code review here: http://codereview.appspot.com/5706047

I added a charset field to dsn and it would look like: /database/user/password[/charset](the charset field is optional).

And I remove the abstract path for more common usage.
The abstract path makes build and test difficult. It always pulls and imports relative packages form your online repo, but my local changed.

Hope this could bring some help.

@ziutek
Copy link
Owner

ziutek commented Feb 29, 2012

Password is intentionally last part of URI to allow '/' character in it (which isn't uncommon). With your modification many applications may not work or stop working after password change and such issue will be difficult to debug.

I think that your problem is database/sql issue and should be fixed in it (there should be something like mymysql Register method, for connection initialization).

@ziutek
Copy link
Owner

ziutek commented Feb 29, 2012

And I remove the abstract path for more common usage.

This breaks some of 'go install' functionality.

@mikespook
Copy link
Author

I think that your problem is database/sql issue and should be fixed in it (there should be something like mymysql Register method, for connection initialization).

Oh, this really makes me dilemma.
The go team don't want to change the interface of database/sql; Put the init-params in the dsn smells really bad.
How about this form: DBNAME+CHARSET/USERNAME/PASSWD

This breaks some of 'go install' functionality.

Could you show more things about this?

@ziutek
Copy link
Owner

ziutek commented Feb 29, 2012

I've added temporary workaround for this issue. Usage:

import "github.com/ziutek/mymysql/godrv"

func init() {
godrv.Register("set names utf8")
}

Notice that this is global for whole application.

Could you show more things about this?

Suppose you write some package/application and publish it as github.com/mikespook/pkg

Someone try install it this way:

go install github.com/mikespook/pkg

'go install' downloads it from github and sees that it imports github.com/ziutek/mymysql/godrv. So next it downloads godrv from github and sees that it imports github.com/ziutek/mymysql/mysql and github.com/ziutek/mymysql/native. So 'go install' downloads them to. Without full path such things are impossible.

@ziutek
Copy link
Owner

ziutek commented Feb 29, 2012

Srorry, I meant old goinstall command (now 'go get'), not 'go install'.

@mikespook
Copy link
Author

I've added temporary workaround for this issue.

Thank you!

But I already added a charset to the dsn, just like this: DBNAME+CHARSET/USERNAME/PASSWD.
I would use this dsn in my projects, until you decide how to deal this issue.

Without full path such things are impossible.

I added a file called import.go in the root dir of mymysql.
You can look it at here: https://github.com/mikespook/mymysql/blob/master/import.go

Then, I could just type go install in the mymysql's dir to install the package.
And if anybody clone this package, do something change, type go test or go build, it will be correct importing form local or same repo.
With the full path, it will import an other repo's codes.

In this case, I modified the native & godrv, then call go test in godrv dir, but nothing happened. After minutes check, I found it imported your repo's native codes.

@ziutek
Copy link
Owner

ziutek commented Mar 1, 2012

I've tried 'go get github.com/mikespook/mymysql/godrv' to see does your way for imports works, but it installs unmodified version because git tags aren't modified.

@mikespook
Copy link
Author

Try "go get github.com/mikespook/mymysql".

@ziutek
Copy link
Owner

ziutek commented Mar 1, 2012

Suppose that I write some application. I publish it on github to be goinstalable. It contains

import github.com/mikespook/mymysql/godrv

so

go get github.com/mikespook/mymysql/godrv

need to work because I import "godrv" not "mymysql". Does your method for imports works for this situation or some user of my application need to manually install github.com/mikespook/mymysq before 'go get myapplication'?

@mikespook
Copy link
Author

I see.
You are on the right track, again.

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