search

pennylane-hq / activerecord-adapter-redshift

Other
6 stars 27 forks source link

Redshift 7.1 Adapter: implement reconnect! like it was in 7.0 #24

Closed jerryclinesmith closed 6 months ago

jerryclinesmith commented 7 months ago

Closes #19

jerryclinesmith commented 7 months ago

All my tests pass, as well as running in a production environment.

quentindemetz commented 7 months ago

@jerryclinesmith why did you close the PR?

jerryclinesmith commented 7 months ago

I was still getting sporadic hard crashes, even with this change.

It's in a job using the Parallel gem, and found an issue where others reporting the same behavior, so trying to figure out if the problem is in the Redshift adapter or the parallel gem, but didn't want you to merge another change unless I was sure it was correct.

quentindemetz commented 7 months ago

Thanks for the explanation

jerryclinesmith commented 7 months ago

So the issues seemed to be with how I was using the parallel gem, and not these changes, so I'm going to reopen the pull request.

Also wanted to let you know, that I have another branch that brings the 7.1 RedshiftAdapter and DatabaseStatements fully up to date with their Postgres counterparts. I'm still testing this branch, as the changes are extensive, but I can make another pull request later if you think you would be interested. I've just brought in all of the latest Postgres code, but left the type maps as they were, and left out code that doesn't apply to Redshift (extensions, enum types, etc...).

jerryclinesmith commented 6 months ago

Going to close this, the other (better IMHO) pull request has been accepted.