search

GoogleCloudPlatform / dialogflow-integrations

Dialogflow integrations with multiple platforms including KIK, Skype, Spark, Twlio, Twitter and Viber
https://cloud.google.com/dialogflow/
Apache License 2.0
256 stars 503 forks source link

Chatbase logging for Twilio integration #25

Closed arcataroger closed 3 years ago

arcataroger commented 4 years ago

Hi there!

I have attempted to add optional Chatbase logging for the twilio integration.

Apologies in advance: This is my first time submitting a pull request, and I'm not sure if I'm doing it right. I'm also not sure if Chatbase is better added on a per-integration basis, or somewhere in botlib, but it seems like it would need to be individually tailored to each vendor's API anyway, so maybe this makes sense...?

I also don't have accounts with the other vendors to test with, unfortunately.

Hope this helps someone else. We use Chatbase to monitor our own Dialogflow agents and it's been helpful.

arcataroger commented 4 years ago

@tyhu-google Sorry for the delay on this. I didn't see this message until just now.

Rather than building a Chatbase integration into every specific provider plugin, might it make more sense to build it into a different part of the lib? I didn't feel comfortable enough doing that for the providers that I didn't have an account for, especially since Dialogflow and Chatbase are both Google products. Are there Googlers already working on integrating the two within Dialogflow itself?

Or is it really OK to just patch this in one provider at a time?

tyhu-google commented 4 years ago

@tyhu-google Sorry for the delay on this. I didn't see this message until just now.

Rather than building a Chatbase integration into every specific provider plugin, might it make more sense to build it into a different part of the lib? I didn't feel comfortable enough doing that for the providers that I didn't have an account for, especially since Dialogflow and Chatbase are both Google products. Are there Googlers already working on integrating the two within Dialogflow itself?

Or is it really OK to just patch this in one provider at a time?

I'm not quite sure which part your comment is referring to. My comment in your CL is asking to move the logic into a helper function in the same file. You don't necessarily to create a separate lib for it?

arcataroger commented 4 years ago

@tyhu-google Sorry for the delay on this. I didn't see this message until just now. Rather than building a Chatbase integration into every specific provider plugin, might it make more sense to build it into a different part of the lib? I didn't feel comfortable enough doing that for the providers that I didn't have an account for, especially since Dialogflow and Chatbase are both Google products. Are there Googlers already working on integrating the two within Dialogflow itself? Or is it really OK to just patch this in one provider at a time?

I'm not quite sure which part your comment is referring to. My comment in your CL is asking to move the logic into a helper function in the same file. You don't necessarily to createe a separate lib for it?

@tyhu-google Sorry tyhu, I should've been clearer.... this is my first time contributing back upstream to an open-source project, and I'm not sure how to best proceed?

I wrote the code this way because it was all we needed for our simple business case, but it seemed to me that such a feature might be better suited elsewhere in the lib because Chatbase isn't particularly provider-centric; it seemed more logical to associate it with the main Dialogflow functions rather than a specific provider integration, but I wasn't sure about that. I had assumed -- probably naively -- that a PR would generate some community discussion, but since it hasn't, I figured I should probably be more proactive/explicit and ask you.

Is it worth patching this PR as you've suggested, or should I pursue a more broadly-compatible request to integrate Chatbase logging into Dialogflow as a whole ,not just with Twilio? And if so, how?

Sorry if this question sounds dumb... still new here, please bear with me :)

arcataroger commented 3 years ago

Closing this PR because Chatbase has been abandoned by Google.

https://web.archive.org/web/20210701153646/https://chatbase.com/documentation/docs-overview