- 
                Notifications
    You must be signed in to change notification settings 
- Fork 176
feat: add cloudnative-pg-mixin #1469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
5052974    to
    8bd7aa7      
    Compare
  
    | Hi! Thanks for contributing. It this a copy cloudnative dashboards and alerts? Would it be more practical to repack their alerts/dashboard as mixin in cloudnative repo instead? So contributors and users can more easily discover it? | 
| Generally speaking, this approach makes sense. However, I'm not very optimistic that such a contribution would be accepted, as the repository appears to be limited to Helm charts: 
 Additionally, I’ve noticed that several mixins within this project have counterparts in their respective upstream repositories. For this reason, I thought it might make sense to include the mixins here. | 
| Hi @arikgrahl Let's give it a shot and see what they say, otherwise happy to add it here for everyone. Perhaps even if they turn down the contribution a compromise can be reached with linking between the two? | 
| Hi @arikgrahl, any updates on this PR? Is this something you were able to contribute upstream or would you still like to have it live here? | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple of questions since I am not familiar with the CNPG:
- Does the operator expose more metrics out of PG or is that left to other projects, i.e.: Prometheus and the like?
- If the answer is yes, would you want to add more health metrics? Commit latencies, slow queries ... etc?
| @Dasomeone, thanks for checking in and for all the feedback! @aalhour, thank you for the review and your questions! 
 I hope this answers your question, but please let me know if you’d like more detail or have a specific definition of “health metrics” for PostgreSQL in mind. | 
| @arikgrahl Like I said originally, we'd prefer an attempt is made to commit this upstream so it lives closer to the codebase at first, but if they're unwilling to accept it we can absolutely store it here. Just hesitant to have it live here from the get-go. | 
As already described in the README.md: