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

[Data Sources] Implement Mixpanel #3275

Closed
wants to merge 8 commits into from
Closed

[Data Sources] Implement Mixpanel #3275

wants to merge 8 commits into from

Conversation

gabrielsebag
Copy link

@ghost ghost added the in progress label Jan 13, 2019
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Thanks! Supporting Mixpanel will be a great addition.

Please see my comments.

@ahelord
Copy link

ahelord commented May 27, 2019

this integration is important.
check CI
please

gabrielsebag and others added 3 commits May 30, 2019 11:25
Co-Authored-By: Arik Fraimovich <arik@arikfr.com>
Co-Authored-By: Arik Fraimovich <arik@arikfr.com>
@gabrielsebag
Copy link
Author

@kravets-levko @arikfr I just committed my changes regarding your comments. I'm still waiting docker to build containers locally in order to test.
I'll close this thread when build is over and when I successfully tested the changes locally.

@gabrielsebag
Copy link
Author

I can't build with docker. I got the following error message:
Service 'worker' failed to build: The command '/bin/sh -c npm run build' returned a non-zero code: 137.

But, even without testing, I think it'll be working anyway.

@kravets-levko
Copy link
Collaborator

@gabrielsebag IIRC exit code 137 means "no enough memory". I don't know anything about your environment, just check that docker has at least 4Gb free memory at build stage (e.g. if you're using MacOS there are some limits for docker VM).

@gabrielsebag
Copy link
Author

Thanks, you were right! I just tested and everything works as expected.

@ericholiveira
Copy link

Any plans on merging this PR on Redash SaaS?

@shahafc
Copy link

shahafc commented Jun 9, 2022

Hey, there is any news about this integration?

@susodapop
Copy link
Contributor

Hey @shahafc thanks for pinging this. I'd like to get this merged ASAP, but it needs some work. In the time since this PR was first opened, we've released a new guide for writing query runners. There are also a lot of merge conflicts to resolve.

@gabrielsebag are you still interested in getting this across the line? Or should we close this PR and open a new PR for others to collaborate?

@susodapop susodapop added the Needs More Information Seen by a team member, not ready for a full review label Jul 3, 2022
@shahafc
Copy link

shahafc commented Jul 3, 2022

Hey @shahafc thanks for pinging this. I'd like to get this merged ASAP, but it needs some work. In the time since this PR was first opened, we've released a new guide for writing query runners. There are also a lot of merge conflicts to resolve.

@gabrielsebag are you still interested in getting this across the line? Or should we close this PR and open a new PR for others to collaborate?

Just to let you know a good workaround is using the "JSON" data source and it works fine :)

@gabrielsebag
Copy link
Author

@gabrielsebag are you still interested in getting this across the line? Or should we close this PR and open a new PR for others to collaborate?

Hey @susodapop unfortunately I can't work on it anymore. I suggest that you open a new PR if you think that's necessary. As @shahafc mentioned, it seems that the JSON data source works well :)

@susodapop
Copy link
Contributor

I understand. Thanks anyway for putting in this effort.

@susodapop susodapop closed this Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Information Seen by a team member, not ready for a full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants