Skip to content

chore(MinIO): add (optional) object storage service #341

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
May 20, 2022
Merged

chore(MinIO): add (optional) object storage service #341

merged 14 commits into from
May 20, 2022

Conversation

guidoiaquinti
Copy link
Contributor

@guidoiaquinti guidoiaquinti commented Mar 30, 2022 •
edited
Loading

Description

This PR adds opt-in object storage service capabilities to the Helm chart.

Why?
We are working to add some new features to PostHog that will rely on an object storage service. Without it those features will not be enabled

Documentation change is available here PostHog/posthog.com#3263

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How has this been tested?

1️⃣ Manual testing:

  • the service is provisioned and it is healthy
  • PostHog pods gets the expected environment variables

2️⃣ CI is ✅

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@guidoiaquinti guidoiaquinti mentioned this pull request Mar 31, 2022
Closed
Guido Iaquinti added 4 commits May 16, 2022 14:34
hazzadous
hazzadous reviewed May 17, 2022
View reviewed changes
hazzadous
hazzadous reviewed May 17, 2022
View reviewed changes
hazzadous
hazzadous approved these changes May 18, 2022
View reviewed changes
Copy link
Contributor

@hazzadous hazzadous left a comment

Choose a reason for hiding this comment

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

Looks good

A kubetest might not be amiss also. I guess we are waiting on landing the code in posthog repo first before getting this in?

@guidoiaquinti
Copy link
Contributor Author

A kubetest might not be amiss also. I guess we are waiting on landing the code in posthog repo first before getting this in?

If we want something e2e yes. Otherwise I could simply test if the API endpoint is there and credential works. I think we could extend the current PostHog django & plugin-server healthcheck to check and then rely here on the HTTP code

@guidoiaquinti guidoiaquinti added the bump minor Updates the chart version by 0.1.0 label May 19, 2022
@guidoiaquinti guidoiaquinti marked this pull request as ready for review May 19, 2022 09:57
hazzadous
hazzadous reviewed May 19, 2022
View reviewed changes
@guidoiaquinti guidoiaquinti merged commit e3da978 into main May 20, 2022
@guidoiaquinti guidoiaquinti deleted the minio branch May 20, 2022 09:43
@guidoiaquinti guidoiaquinti mentioned this pull request May 23, 2022
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@hazzadous hazzadous hazzadous approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
bump minor Updates the chart version by 0.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@guidoiaquinti @hazzadous