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

fix(startup): posthog issue when enabled and no connectivity #631

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

vijay-jangir
Copy link
Contributor

Dockerfile enhancement, posthog bugfix

posthog doesn't let application start if there is any connection issue, temporarily fixed it to atleast get the application in started mode.
Updated dockerfile for ui and api so that it can run seemlessly on openshift update dockerfile for api for caching layers more efficiently

Closes #

📑 Description

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

Copy link

vercel bot commented Dec 15, 2023

@vijay-jangir is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (424d3d1) 68.04% compared to head (89f53b2) 68.07%.
Report is 1 commits behind head on main.

❗ Current head 89f53b2 differs from pull request most recent head 08aa3c8. Consider uploading reports for the commit 08aa3c8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #631      +/-   ##
==========================================
+ Coverage   68.04%   68.07%   +0.03%     
==========================================
  Files          48       45       -3     
  Lines        2691     2390     -301     
==========================================
- Hits         1831     1627     -204     
+ Misses        860      763      -97     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… issue, temporarily fixed it to atleast get the application in started mode.

Updated dockerfile for ui and api so that it can run seemlessly on openshift
update dockerfile for api for caching layers more efficiently
@vijay-jangir vijay-jangir changed the title fix: posthog doesn't let application start if there is any connection… fix: posthog startup issue when enabled and no connectivity Dec 15, 2023
Signed-off-by: talboren <talboren2@gmail.com>
@talboren
Copy link
Member

@vijay-jangir I've edited the EventCaptureMiddleware to disable posthog completely unless otherwise configured since we're not really using it and the way you fixed it would've still caused the API to behave strange.

Signed-off-by: talboren <talboren2@gmail.com>
@talboren
Copy link
Member

@vijay-jangir this now lgtm, lmk once you've had a chance to take a look and I can merge this in.

@talboren talboren self-requested a review December 17, 2023 08:43
Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

LGTM

@talboren talboren changed the title fix: posthog startup issue when enabled and no connectivity fix(startup): posthog issue when enabled and no connectivity Dec 17, 2023
@vijay-jangir
Copy link
Contributor Author

This looks fine, as long as we're okay with posthog still crashing the backend if network is behind a proxy or firewall, incase the POSTHOG_ENABLE is set to true.

@talboren
Copy link
Member

talboren commented Dec 19, 2023

This looks fine, as long as we're okay with posthog still crashing the backend if network is behind a proxy or firewall, incase the POSTHOG_ENABLE is set to true.

It's now disabled by default, so it won't :)

@talboren talboren merged commit 7d91b19 into keephq:main Dec 19, 2023
3 of 4 checks passed
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.

2 participants