-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Send proper server parameters to clients using admin db #103
Conversation
src/client.rs
Outdated
@@ -313,8 +313,10 @@ where | |||
|
|||
let mut target_pool: ConnectionPool = ConnectionPool::default(); | |||
let mut transaction_mode = false; | |||
let mut server_info = BytesMut::default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get a warning here because BytesMut::default()
value was overwritten before it was used.
Open for ideas on how to refactor this to avoid the warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make if blocks return values, e.g.
let value = if true {
5
}
else
{
6
}
so you don't have to have a default value up one scope level like in C/C++.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We'll keep iterating and fixing warnings/bugs as they come up. We are comfortable having code in main that's experimental and once it's stable enough, we can cut a release.
src/messages.rs
Outdated
|
||
pub fn server_paramater_message(key: &str, value: &str) -> Result<BytesMut, Error> { | ||
let mut server_info = BytesMut::new(); | ||
if !key.is_ascii() || !value.is_ascii() || key.is_empty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're generating these manually so you don't need this check.
* Send proper server parameters to clients using admin db * clean up * fix python test * build * Add python * missing & * debug ls * fix tests * fix tests * fix * Fix warning * Address comments
Psycopg2 threw an error at connect-time when connecting to admin db, probably because we send no
S
message back for that db (server_info is empty).The error it produced looks like this
This is easily reproducible so I added a test for it and enabled python tests.
This PR generates such message on-the-fly for admin db (I think this cleaner than just grabbing a
server_info
struct from any pool.