lidavidm commented on code in PR #2728:
URL: https://github.com/apache/arrow-adbc/pull/2728#discussion_r2053650664
##########
go/adbc/driver/snowflake/snowflake_database.go:
##########
@@ -156,284 +162,300 @@ func (d *databaseImpl) SetOptions(cnOptions
map[string]string) error {
}
}
- dv, _ := d.DriverInfo.GetInfoForInfoCode(adbc.InfoDriverVersion)
- driverVersion := dv.(string)
- defaultAppName := "[ADBC][Go-" + driverVersion + "]"
// set default application name to track
// unless user overrides it
- d.cfg.Application = defaultAppName
+ d.cfg.Application = d.defaultAppName
- var err error
for k, v := range cnOptions {
v := v // copy into loop scope
- switch k {
- case adbc.OptionKeyUsername:
- d.cfg.User = v
- case adbc.OptionKeyPassword:
- d.cfg.Password = v
- case OptionDatabase:
- d.cfg.Database = v
- case OptionSchema:
- d.cfg.Schema = v
- case OptionWarehouse:
- d.cfg.Warehouse = v
- case OptionRole:
- d.cfg.Role = v
- case OptionRegion:
- d.cfg.Region = v
- case OptionAccount:
- d.cfg.Account = v
- case OptionProtocol:
- d.cfg.Protocol = v
- case OptionHost:
- d.cfg.Host = v
- case OptionPort:
- d.cfg.Port, err = strconv.Atoi(v)
- if err != nil {
- return adbc.Error{
- Msg: "error encountered parsing Port
option: " + err.Error(),
- Code: adbc.StatusInvalidArgument,
- }
- }
- case OptionAuthType:
- d.cfg.Authenticator, ok = authTypeMap[v]
- if !ok {
- return adbc.Error{
- Msg: "invalid option value for " +
OptionAuthType + ": '" + v + "'",
- Code: adbc.StatusInvalidArgument,
- }
- }
- case OptionLoginTimeout:
- dur, err := time.ParseDuration(v)
- if err != nil {
- return adbc.Error{
- Msg: "could not parse duration for '"
+ OptionLoginTimeout + "': " + err.Error(),
- Code: adbc.StatusInvalidArgument,
- }
- }
- if dur < 0 {
- dur = -dur
- }
- d.cfg.LoginTimeout = dur
- case OptionRequestTimeout:
- dur, err := time.ParseDuration(v)
- if err != nil {
- return adbc.Error{
- Msg: "could not parse duration for '"
+ OptionRequestTimeout + "': " + err.Error(),
- Code: adbc.StatusInvalidArgument,
- }
- }
- if dur < 0 {
- dur = -dur
+ err := d.SetOptionInternal(k, v, &cnOptions)
+ if err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// SetOptionInternal sets the option for the database.
+//
+// cnOptions() is nil is SetOptionInternal() is being called when setting an
+// option post-initialization.
Review Comment:
Can you reword this? It's not immediately obvious what it's trying to say.
(`cnOptions is nil if the option is being set post-initialiation.` perhaps)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
