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

2116: improve resilience of posthog integration #152

Merged
merged 29 commits into from
Apr 24, 2024

Conversation

collinpreston
Copy link
Contributor

@collinpreston collinpreston commented Apr 23, 2024

What are the relevant tickets?

mitodl/mitxonline#2116

Description

Defines the generic Posthog functionality found in MITx Online and MIT Open to ol-django. This ol-django functionality will later be imported into MIT Open and MITx Online in order to reduce redundant code across our applications.

How can this be tested?

You can use this MITx Online branch to test the functionality provided by this PR: https://github.com/mitodl/mitxonline/tree/ol-django-posthog

Test setup

You will need to perform the following steps in order to test this PR using the MITx Online branch above.

  1. From within the ol-django project, run pants package "src/mitol/olposthog:"
  2. Copy the file from dist/mitol-django-olposthog-2024.4.16.tar.gz and paste it into the main directory of the MITx Online application.
  3. From within the MITx Online application's root directory, run docker-compose run -u root --rm web poetry add ./mitol-django-olposthog-2024.4.16.tar.gz.
  4. From the same location as step 3, run docker-compose run -u root --rm web poetry install.
  5. From the same location as step 4, run docker-compose build.
  6. Follow the "Installation and setup" section of the README included with this PR.
  7. From within the MITx Online application's root directory, run docker-compose up.
  8. Verify that your local MITx Online application appears as it should when POSTHOG_ENABLED=True is defined in your .env file.
  9. Verify that your local MITx Online application appears as it should when POSTHOG_ENABLED=False is defined in your .env file.

@collinpreston collinpreston linked an issue Apr 23, 2024 that may be closed by this pull request
@collinpreston
Copy link
Contributor Author

@rhysyngsun I don't understand why

def check(ctx: Context, project: Project, base: str, target: str):
is reporting that I need to make change logs for a bunch of apps that I didn't modify. That doesn't seem right.

Copy link
Contributor

@jkachel jkachel left a comment

Choose a reason for hiding this comment

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

Code looks OK and the automated tests pass. Have a couple small things that need looking at (just docs) but should be OK after that - these are just doc things so no don't think it requires a re-review.

#### Retrieve all feature flags from Posthog
You can retrieve all the feature flags from Posthog using:
```
from mitol.olposthog.features import is_enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Small thing - import needs to be updated

@@ -0,0 +1,34 @@
.. A new scriv changelog fragment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Small thing - should this have a log entry?

@collinpreston collinpreston merged commit 14a8693 into main Apr 24, 2024
8 checks passed
@collinpreston collinpreston deleted the 2116-improve-resilience-of-posthog-integration-1 branch April 24, 2024 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve resilience of posthog integration
2 participants