Support target session attrs #810
Conversation
bpicolo
force-pushed
the
support_target_session_attrs
branch
4 times, most recently
from
c68363e to
20c8b4d
Compare
bpicolo
force-pushed
the
support_target_session_attrs
branch
from
20c8b4d to
43cf9fb
Compare
| panicking = false | ||
| return cn, err | ||
|
|
||
| // In most cases |
There was a problem hiding this comment.
Forgot to finish this sentence.
There's some question about how we would want to return errors/ connections here. Currently this takes the path of least resistance and uses the latest connection / err
|
Maybe I'm missing something, but why couldn't this have been done as an improvement to #714? Seems a bit impolite to take my initial code and just squash it under your name. |
|
@costela probably should be! Happy to close this. I needed it myself and am not used to PRing other people’s forked PRs and GitHub doesn’t provide the best social interactivity there =]. Happy to figure out how to integrate / close this with you. Definitely appreciate the work you put in. Didn’t intend to ignore the work you put in, which is why I linked your PR in the initial comment. Sorry that I didn’t do the best job there. Getting the maintainer notion of “I don’t plan to look at this any time soon” put me in a pickle of needing to pull things out I actually hadn’t really intended to put this PR in the main repo at all, but sort of just edited the comment after I’d noticed I unintentionally had |
|
Sure, totally understandable. Thanks for the clarification! And if it turns out we have fundamentally different ideas about how this feature should work, you can always open a separate PR in the future again. |
This is a remake of #714 that pulls in much related code, but also has a couple bugfixes for SSL tests, and updates some related documentation (for how to run multi-pg tests locally).