Closed francois-gay closed 3 years ago
Hi @francois-gay, I think this would be a matter for SQLAlchemy.
The way the PostgreSQL protocol works is that the password has to be sent to the server as bytes, but it has to be sent before the client knows the encoding. So pg8000 accepts a password as a string or bytes object. If it's a string pg8000 encodes the string using UTF8.
If SQLAlchemy wants to support any object with a __str__
method then SQLAlchemy can call str(password)
before passing it on to pg8000. Does that make sense?
Hi @tlocke.
Yes,it makes sense, but it could be useful for all other users.
It could be done just by adding 2 lignes here : https://github.com/tlocke/pg8000/blob/c0b9a44e366686213e8933da6d377ccbf93e0287/pg8000/core.py#L195
if isinstance(password, str):
self.password = password.encode("utf8")
elif isinstance(password, bytes) or password is None:
self.password = password
else:
self.password = str(password).encode("utf8")
Don't you think so ?
With things like this I tend to be mindful of The Zen Of Python, particularly the 2nd and 3rd:
Explicit is better than implicit. Simple is better than complex.
So my thinking is that this change would involve doing things implicitly and also it would be make the connect
function a bit more complex to understand. I also tend to be guided by Liedtke's minimality principle:
A concept is tolerated inside the microkernel only if moving it outside the kernel, i.e., permitting competing implementations, would prevent the implementation of the system's required functionality.
Liedtke was talking about microkernels, but I think the same sort of thing applies to libraries such as pg8000. In this case the user of pg8000 can do the str(password)
operation, so according to the minimality principle it shouldn't be inside pg8000.
Anyway, those are my thoughts at the moment, but everything's always up for debate :-)
I agree with you, and open an issue in SQLAlchemy Github (so... as a bug :) ).
Thank you for your point of view and your answers.
Corrected in SQLAlchemy
SQLAlchemy says about password:
URL.password: password, which is normally a string but may also be any object that has a __str__() method.
Could you implement that in a future version ?