feat: Add PostgreSQL Retriever #2790
Conversation
✅ Deploy Preview for go-feature-flag-doc-preview canceled.
|
gogbog
force-pushed
the
feat/add-postgresql-receiver
branch
from
56bc75d to
6ea1e93
Compare
There was a problem hiding this comment.
Hey @gogbog this is a super nice start, I have left some small nit comments.
As we said in slack I would also like to have the structured version too in the same PR if possible. WDYT?
| r.logger.Error("Flag key does not have a string value") | ||
| } | ||
| } else { | ||
| r.logger.Error("No 'flag' entry found") |
There was a problem hiding this comment.
🤓 nitpick: I am not sure that it is really an error to have no flag.
| r.logger.Error("No 'flag' entry found") | |
| r.logger.Warn("No 'flag' entry found") |
| if r.dbPool == nil { | ||
| r.status = retriever.RetrieverNotReady | ||
|
|
||
| poolConfig, err := pgxpool.ParseConfig(r.URI) |
There was a problem hiding this comment.
❔ question: Do we need a connection pool here? We will have only 1 connection per retriever, I am not super familiar with pgxpool but what value is it bringing here?
There was a problem hiding this comment.
You are right. Changed it to direct connection instead of a pool. If we have single connection it won't bring any value.
| return err | ||
| } | ||
|
|
||
| pool, err := pgxpool.NewWithConfig(ctx, poolConfig) |
There was a problem hiding this comment.
❔ question: Same question for pgxpool.
| } | ||
| defer rows.Close() | ||
|
|
||
| ffDocs := make(map[string]interface{}) |
There was a problem hiding this comment.
🎯 suggestion: it is not super explicit to me what ffDocs contains. I can't come with a better name, but it does not sound self explain what do we store in this map.
There was a problem hiding this comment.
Yup, it's not the best variable name. I've updated it to mappedFlagDocs as it feels more self-explanatory. I'm still not sure what name would be the best, If you have better suggestions, let me know.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2790 +/- ##
==========================================
- Coverage 84.77% 84.66% -0.11%
==========================================
Files 111 112 +1
Lines 5207 5289 +82
==========================================
+ Hits 4414 4478 +64
- Misses 628 641 +13
- Partials 165 170 +5 ☔ View full report in Codecov by Sentry. |
Description
This PR adds a PostgreSQL retriever for Goff. For example, we use PostgreSQL in our projects, and it would be nice to stick to one database for everything. Most likely, there are other people who would benefit from this as well.
Implementation Details:
New Libraries:
Current Retriever Options:
JSON. In the future, we can extend this to support a more structured table approach.@thomaspoignant , happy to share this and open to improving it further if you have any feedback!
Checklist
examplesfolder of how to run the providerREADME.mdand/website/docs)