user pool_size takes precedence over database pool_size.#781
user pool_size takes precedence over database pool_size.#781levkk merged 3 commits intopgdogdev:mainfrom
Conversation
User is more specific, and you have multiple users per database, so it's kinda surprising that the database pool size takes precedence. Fix that.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
mijoharas
force-pushed
the
fix-order-of-max-database-config
branch
from
107c3aa to
546fab2
Compare
|
It was not intentional, and kind of became the norm since changing this is breaking. And that's fine. I'll put a note in the release notes, because this is the right way to handle it. Could you quickly take a look at any other settings we set there and make sure user overrides database there as well? Then we can merge. I can update the docs (or if you're willing to make a PR there as well). Thank you! |
|
hold up, saw some other ones, let me fix them too. (glad I still had the file open in my editor) |
|
Alright, should have it all now. (I simplified the statement_timeout branching a little, otherwise everything is a mechanical user/database swap). |
| } | ||
|
|
||
| #[test] | ||
| fn test_user_takes_precedence_over_database() { |
There was a problem hiding this comment.
Not a terrible idea to do the other configs too. Only the paranoid survive, something something
There was a problem hiding this comment.
good call, let me do that.
There was a problem hiding this comment.
ok had a meeting but just got back to it. Have pushed the extra test commit.
|
Okay let's see if the tests catch anything. If we're all green, we know are tests don't cover enough! 😆 |
hahaha, they were when I ran them locally 😅 |
2 participants
User is more specific, and you have multiple users per database, so it's kinda surprising that the database pool size takes precedence.
Fix that.
This one caught us out, I'm assuming it's not intentional? if so, it's a bit confusing and maybe the docs should outline that? (I still think it should be changed though.)