search

amphp / mysql

An async MySQL client for PHP, optimizing database interactions with efficient non-blocking capabilities. Perfect for responsive, high-performance applications.
Other
358 stars 63 forks source link

Add connect context arg to Connection::connect() #82

Closed trowski closed 5 years ago

trowski commented 6 years ago

Addresses #81.

trowski commented 6 years ago

@prolic @kelunik Please review.

prolic commented 6 years ago

Looks good to me. I didn't check for postgres, maybe it's the same problem there? Can you also add a test?

trowski commented 6 years ago

@kelunik As far as I can tell the previous way worked too (edit: nope, cancellation isn't checked until the socket timeout expires), though this way is better since a timeout >10 sec would have been overridden by the socket context.

@prolic I don't think this is a problem in postgres since that library uses loop watchers for connection due to how the extensions work.

bwoebi commented 6 years ago

Shouldn't TimeoutConnector gain that new context arg as well?

trowski commented 6 years ago

Potentially, but I think it would be better to add another class such as SocketContextConnector and keep TimeoutConnector simple.

kelunik commented 5 years ago

Any reason why this has not been merged?