You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2024/03/07 17:48:26 UTC
[PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
lidavidm opened a new pull request, #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594
Fixes #1588.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594#discussion_r1516630595
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -64,10 +64,16 @@ func doGet(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEnd
}
var (
- cc interface{}
+ cc interface{}
+ hasFallback bool
)
for _, loc := range endpoint.Location {
+ if loc.Uri == flight.LocationReuseConnection {
+ hasFallback = true
+ continue
+ }
Review Comment:
shouldn't we `break` instead of `continue`? or at least do the same as we do below and attempt to reuse the connection, continuing the loop if it errors, returning if it succeeds?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594#discussion_r1516634539
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -64,10 +64,16 @@ func doGet(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEnd
}
var (
- cc interface{}
+ cc interface{}
+ hasFallback bool
)
for _, loc := range endpoint.Location {
+ if loc.Uri == flight.LocationReuseConnection {
+ hasFallback = true
+ continue
+ }
Review Comment:
is that correct semantically? We don't want to maintain the order and have it do the reuse-connection before moving on to further locations?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594#discussion_r1516637015
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -64,10 +64,16 @@ func doGet(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEnd
}
var (
- cc interface{}
+ cc interface{}
+ hasFallback bool
)
for _, loc := range endpoint.Location {
+ if loc.Uri == flight.LocationReuseConnection {
+ hasFallback = true
+ continue
+ }
Review Comment:
I think it's always been said that order is unspecified.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594#discussion_r1516631607
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -64,10 +64,16 @@ func doGet(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEnd
}
var (
- cc interface{}
+ cc interface{}
+ hasFallback bool
)
for _, loc := range endpoint.Location {
+ if loc.Uri == flight.LocationReuseConnection {
+ hasFallback = true
+ continue
+ }
Review Comment:
What I'm doing is: first try everything except reuse-connection, then if all other alternatives fail, try the original service
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(go/adbc/driver/flightsql): support reuse-connection location [arrow-adbc]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1594:
URL: https://github.com/apache/arrow-adbc/pull/1594#discussion_r1516664449
##########
go/adbc/driver/flightsql/flightsql_connection.go:
##########
@@ -64,10 +64,16 @@ func doGet(ctx context.Context, cl *flightsql.Client, endpoint *flight.FlightEnd
}
var (
- cc interface{}
+ cc interface{}
+ hasFallback bool
)
for _, loc := range endpoint.Location {
+ if loc.Uri == flight.LocationReuseConnection {
+ hasFallback = true
+ continue
+ }
Review Comment:
Okay then, I'm fine with this :)
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org