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

Enable SSL PostgreSQL configuration through env variables #2967

Merged
merged 4 commits into from
Jan 19, 2021

Conversation

tmilicic
Copy link
Contributor

Changes

Please describe.
If this affects the frontend, include screenshots.

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@Twixes Twixes changed the title Enable SLL PostgreSQL configuration through env variables Enable SSL PostgreSQL configuration through env variables Jan 18, 2021
@Twixes
Copy link
Member

Twixes commented Jan 18, 2021

Hey! Thanks a lot for the contribution, we'll work on merging this. Are you a Postgres-over-SSL user yourself?

@Twixes Twixes requested a review from EDsCODE January 18, 2021 11:23
@tmilicic
Copy link
Contributor Author

Hey! Thanks a lot for the contribution, we'll work on merging this. Are you a Postgres-over-SSL user yourself?

Hey @Twixes, thank you so much!
Yes, I am currently working on setting up Postgres-over-SSL as part of the project for the company I work in. This would really help us and save us some additional code.

@Twixes
Copy link
Member

Twixes commented Jan 18, 2021

As a side note for our team, after we accept this, but before we merge it, we're going to have to implement the same in https://github.com/PostHog/plugin-server for event processing to work with the same configuration.

Copy link
Member

@EDsCODE EDsCODE left a comment

Choose a reason for hiding this comment

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

Hi @tmilicic, thanks for the contribution! Before accepting this, would you mind adding relevant docs that accompany this change on our docs repo?

@tmilicic
Copy link
Contributor Author

Hi @tmilicic, thanks for the contribution! Before accepting this, would you mind adding relevant docs that accompany this change on our docs repo?

Hi @EDsCODE, oppened the PR with relevant docs here

@EDsCODE
Copy link
Member

EDsCODE commented Jan 18, 2021

Awesome, thanks.

@Twixes do you mind making the addition to the plugin-server? Otherwise, I can pick it up in a short bit

@tmilicic
Copy link
Contributor Author

Hi, is there something I can do to push this foreward? I saw that docs for this have already been merged

@Twixes
Copy link
Member

Twixes commented Jan 19, 2021

Hey, so at the moment the plugin server supports connections exclusively using the DATABASE_URL env var, which does support these settings (see brianc/node-postgres#2345), so for me this PR is a go. Just awaiting confirmation from @EDsCODE.
Should you experience any issues @tmilicic with any piece of PostHog not cooperating with Postgres over SSL, let us know.
Disregard the single failing Cypress test causing redness, it's an unrelated problem. #2988

Copy link
Member

@EDsCODE EDsCODE left a comment

Choose a reason for hiding this comment

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

lgtm!

@EDsCODE EDsCODE merged commit c1bdc0f into PostHog:master Jan 19, 2021
@yakkomajuri
Copy link
Contributor

Hey @tmilicic! Thanks for the PR - email me at yakko[at]posthog[.]com from your GH email to get some merch from us!

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

Successfully merging this pull request may close these issues.

4 participants