create global default privileges in the appropriate prepared databases #1421
Conversation
machine424
requested review from
CyberDem0n,
FxKu,
Jan-M,
RafiaSabih,
erthalion and
sdudoladov
as code owners
pkg/cluster/sync.go
Outdated
There was a problem hiding this comment.
Can we remove this, this does not do anything and imho the comment is also not correct, the defer part comes post init.
There was a problem hiding this comment.
the defer part comes post init I didn't get it?
I was trying to say that, in case we enter in the loop in line 666, the first initDbConnWithName in line 667 will try to close the connection we open in line 612 and that this defer will close the last connection that we open in line 667 so we don't need to add another close.
I'll remove it.
There was a problem hiding this comment.
maybe remove it if it confuses Jan ;)
I think from the code it should be clear that the last connection opened in this method will be closed in the defer part.
There was a problem hiding this comment.
yep, and to underline the fact that initDbConnWithName (tries to) does close the conn before creating another one so we don't risk a conn risk.
Done.
machine424
force-pushed
the
cata
branch
2 times, most recently
from
c91b7dd to
279d96f
Compare
|
👍 |
1 similar comment
|
👍 |
|
Thank you @machine424 |
3 participants
fixes #1420
I can see the default privileges now.
I don't see how to easily test this, maybe the preparedDatabases feature needs some e2e tests?