Experimental PG patch

[tedaford]

This PR will add PostGreSQL support to Faulty as a patch. Verified to work using the unit tests of my humble anonymous enterprise web app.

[justinhoward]

This would be a great addition!

[tedaford]

Seems to work, at least on the limited example of 1 enterprise-level web application backend. Suggestions and comments are welcome.

[codecov]

Codecov Report

Base: 98.76% // Head: 97.08% // Decreases project coverage by -1.67% ⚠️

Coverage data is based on head (811b0c3) compared to base (3eaf01c).
Patch has no changes to coverable lines.

❗ Current head 811b0c3 differs from pull request most recent head 0401966. Consider uploading reports for the commit 0401966 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
- Coverage   98.76%   97.08%   -1.68%     
==========================================
  Files          35       35              
  Lines        1134     1134              
  Branches      159      159              
==========================================
- Hits         1120     1101      -19     
- Misses         14       33      +19     

Impacted Files	Coverage Δ
lib/faulty/patch/mysql2.rb	4.34% <0.00%> (-82.61%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[justinhoward]

Thanks for your contribution! Since faulty is a reliability tool, there's a few things I want to be sure of with this addition:

• Have we considered all the methods that connect to the database? Which ones do we want to be wrapped in a circuit (or not)?
• With MySQL, COMMIT and ROLLBACK were the only statements we needed to exclude from the circuit. With PostgreSQL, are there any other statements that should be excluded?
• Need integration tests. We should be able to do something similar to MySQL.
• We have fairly complete API documentation coverage for the codebase in addition to the guides in the README. So we should continue that here, so we make sure to cover usage details, etc in those docs as well.

For the first item, I can take a look at that a bit as well, but it might take me some time to do a thorough review. Thanks again!

[tedaford]

Sounds good! I will research more as I am able;

methods that connect to database
In my case, I tested it on a ruby project that was connecting to one DB directly using the PG gem, and one using ActiveRecord. I wrapped ActiveRecord separately.

statements that need excluding
I don't think BEGIN counts, COMMIT and ROLLBACK would need to be excluded, otherwise (I'm not a super Postgres expert) I think that's it

integration tests
I absolutely agree

docs
Again, I absolutely agree, I kind of take the position that the "code describes the comments to the machine", vs the other way around, documentation is at least as important as the code itself.

[justinhoward]

It looks like you may have accidentally removed the mysql2 dependency in the Gemfile, which causes the mysql specs to be skipped. Your tests for PG are also not running on CI. You'll need to add pg to the Gemfile as well, and make sure the patch is required in spec_helper.

You'll see that we have to conditionally test mysql2 depending on the ruby version (we specifically don't support the mysql2 patch on Ruby 2.3). I'm guessing that's not necessary here.

[tedaford]

(ah, crud, I did do that, I was having trouble at the time getting mysql to build while testing locally and didn't use a branch on my fork- as an aside, everything else seems to work fine in arm64-based linux)
Will fiddle with things later as I get time

[justinhoward]

@tedaford Did you mean to close this? If you're no longer interested in pursuing this, there's no pressure of course. I may continue this work in the future since I do think this would be a valuable addition. Either way, thanks for your work!

[tedaford]

I meant to try to make it a "draft" PR to keep it from banging on the unit tests, if it's no bother I'll leave it- working on a branch right now