On Fri, Dec 20, 2024 at 2:21 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 20 Dec 2024, at 02:00, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>
> Thanks for the new version, I was doing a v39 review but I'll roll that over
> into a v40 review now.
(Sorry for the rug pull!)
> As I was reading I was trying to identify parts can be broken out and committed
> ahead of time. This not only to trim down size, but mostly to shape the final
> commit into a coherent single commit that brings a single functionality
> utilizing existing APIs. Basically I think we should keep generic
> functionality out of the final commit and keep that focused on OAuth and the
> required APIs and infra.
Sounds good.
> The async auth support seemed like a candidate to go in before the rest. While
> there won't be any consumers of it, it's also not limited to OAuth. What do
> you think about slicing that off and get in ahead of time? I took a small stab
> at separating out the generic bits (it includes the PG_MAX_AUTH_TOKEN_LENGTH
> move as well which is unrelated, but could also be committed ahead of time)
> along with some small tweaks on it.
+1 to separating the PG_MAX_... macro move. I will take a closer look
at the async patch in isolation; there's some work I'm doing to fix a
bug Kashif (cc'd) found recently, and it has me a bit unsure about my
chosen order of operations in the async part of fe-connect.c. That
deserves its own email, but I need to investigate more.
Thanks!
--Jacob
