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 Predis integration for clusters #574

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

RaoulMeyer
Copy link
Contributor

@RaoulMeyer RaoulMeyer commented Sep 14, 2019

Description

Currently, the Predis integration makes our application crash when enabling APM (see issue #315). Because the interface that Predis provides doesn't allow getting the connection details for a cluster, in this PR I've removed the host and port tags for clusters. This also makes the retrieving of host and port a bit cleaner.

Readiness checklist

  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.
  • Changelog has been added to the appropriate release draft. For community contributors the reviewer is in charge of this task.

@RaoulMeyer RaoulMeyer force-pushed the fix-predis-cluster-integration branch 3 times, most recently from d6002bb to f68ef92 Compare September 14, 2019 15:16
@RaoulMeyer RaoulMeyer force-pushed the fix-predis-cluster-integration branch from f68ef92 to af60be3 Compare September 14, 2019 15:17
@SammyK
Copy link
Contributor

SammyK commented Sep 16, 2019

Thank you so much for this fix @RaoulMeyer! I'll make sure we get this reviewed this week so that we can roll it into a release as soon as possible. :)

@SammyK SammyK added 🏘 community Contributions from the general community 🐛 bug Something isn't working labels Sep 16, 2019
@SammyK SammyK added this to the 0.31.0 milestone Sep 20, 2019
Copy link
Contributor

@SammyK SammyK left a comment

Choose a reason for hiding this comment

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

Excellent work @RaoulMeyer! Thank you so much for taking the time to submit a PR. We will get this rolled into the next release. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🏘 community Contributions from the general community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants