Skip to content
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

Fixes to growthbook chart to allow use of AWS DocumentDB #160

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

AdamStaudt
Copy link

@AdamStaudt AdamStaudt commented May 2, 2023

Proposed changes

This PR contains changes which make it possible to apply the growthbook chart to configure the deployment to use an AWS DocumentDB cluster as its MongoDB backend.

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

@AdamStaudt AdamStaudt changed the title Fixes to use aws documentdb Fixes to growthbook chart to allow use of AWS DocumentDB May 2, 2023
@bisonlou
Copy link
Contributor

@AdamStaudt Thanks for this PR. Could you kindly update the version number before review?

@AdamStaudt
Copy link
Author

@AdamStaudt Thanks for this PR. Could you kindly update the version number before review?

version number updated.

version: ~13.6.1
repository: https://charts.bitnami.com/bitnami/
condition: mongodb.enabled
# dependencies:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdamStaudt great work, I just wonder why you would remove the Mongodb dependency. I just think it should be disabled by default instead. That way it will not be installed for those who do not need it. For those who need it, they could always override the switch

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, it should be diabled by default instead, rather than removed entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants