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

support logger when querying database #8

Closed
wants to merge 4 commits into from
Closed

Conversation

nullne
Copy link
Contributor

@nullne nullne commented Apr 5, 2021

No description provided.

@coveralls
Copy link

coveralls commented Apr 5, 2021

Coverage Status

Coverage decreased (-1.2%) to 86.782% when pulling 39066c3 on nullne:master into 34fba28 on linxGnu:master.

@linxGnu
Copy link
Owner

linxGnu commented Apr 6, 2021

@nullne IMHO, I think mssqlx should be as thin and dependency-less as possible.

In this way, I think we should not inject logging this way. How about letting user overwrite DB instantiation. For example:

type Instantiate func(driver, dsn string) (*sql.DB, error)

type clusterOptions struct {
   ...
   instantiate Instantiate
}

func WithInstantiate(f Instantiate) Option {
        return func(o *clusterOptions) {
                o.instantiate = f
        }
}
var db *sql.DB
if opts.instantiate != nil {
        db, err = opts.instantiate(driverName, masterDSNs[mId])
} else {
        db, err = sql.Open(driverName, masterDSNs[mId])
}

if err != nil {
    return
}

dbConn := sqlx.NewDb(db, driverName)

In this way, user could instrument db instantiated as he wishes in anyway he want:

db, err := mssqlx.Open(...., WithInstantiate(driver, dsn string) *sql.DB {
    logger := ....
     ...
     db := sqldblogger.OpenDriver(
					masterDSNs[mId],
					dbConn.Driver(),
					logger, loggerOptions...
      )
      return db
})

@linxGnu
Copy link
Owner

linxGnu commented Apr 7, 2021

@nullne ptal: #9

@nullne
Copy link
Contributor Author

nullne commented Apr 9, 2021

it works, thank you very much @linxGnu

@nullne nullne closed this Apr 9, 2021
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.

None yet

3 participants