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

feat(integrations): add support for Amplitude base url #2534

Merged
merged 5 commits into from
Jul 31, 2023

Conversation

matthewelwell
Copy link
Contributor

@matthewelwell matthewelwell commented Jul 27, 2023

Changes

Adds support for base_url attribute in the AmplitudeConfiguration model.

Fixes #2522

NOTE: we will also need to update the integration_data flag to expose this in the UI.

How did you test this code?

Added unit test. I've also tested this manually to verify that with the new default url (api2.amplitude.com) events are still ingested correctly.

@matthewelwell matthewelwell requested review from a team and novakzaballa July 27, 2023 13:17
@vercel
Copy link

vercel bot commented Jul 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 27, 2023 3:38pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 27, 2023 3:38pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 27, 2023 3:38pm

@github-actions github-actions bot added the api Issue related to the REST API label Jul 27, 2023
@matthewelwell matthewelwell changed the title feature: add support for Amplitude base url feat(integrations): add support for Amplitude base url Jul 27, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 27, 2023

Uffizzi Preview deployment-31951 was deleted.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (d7c459e) 95.43% compared to head (4495768) 95.44%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2534   +/-   ##
=======================================
  Coverage   95.43%   95.44%           
=======================================
  Files         972      974    +2     
  Lines       27191    27207   +16     
=======================================
+ Hits        25951    25967   +16     
  Misses       1240     1240           
Files Changed Coverage Δ
api/integrations/amplitude/amplitude.py 87.50% <100.00%> (-0.50%) ⬇️
api/integrations/amplitude/constants.py 100.00% <100.00%> (ø)
.../amplitude/migrations/0006_add_default_base_url.py 100.00% <100.00%> (ø)
api/integrations/amplitude/models.py 100.00% <100.00%> (ø)
api/integrations/amplitude/serializers.py 100.00% <100.00%> (ø)
...unit/integrations/amplitude/test_unit_amplitude.py 100.00% <100.00%> (ø)

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

Copy link
Member

@khvn26 khvn26 left a comment

Choose a reason for hiding this comment

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

Leaving this comment as a reminder to update Edge API too.

@matthewelwell
Copy link
Contributor Author

Leaving this comment as a reminder to update Edge API too.

Yep, good point @khvn26. Edge PR is here: #2534 (review).

@matthewelwell matthewelwell merged commit 5d52564 into main Jul 31, 2023
16 checks passed
@matthewelwell matthewelwell deleted the feature/add-amplitude-base-url branch July 31, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Base URL to Amplitude integration
4 participants