Problem/Motivation
Dialogflow has changed completely the way entities are exposed and managed through the API V2.
Proposed resolution
We need to align with the changes, if we want to keep providing this feature.
Remaining tasks
CryWipe the tearsSuggest - not require - the google/cloud-dialogflow package on our composerUse the library to sync entities- Update documentation, mainly about authentication as it's crazy.
Release notes snippet
TBD
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3082765-support-v2-entities-sync-9.patch | 19.81 KB | gambry |
| #8 | 3082765-support-v2-entities-sync-7.patch | 19.78 KB | gambry |
| #7 | interdiff--4-7.txt | 15.3 KB | gambry |
Comments
Comment #2
larowlan😂
Comment #3
gambryYeah dude. :(
Working on this BTW.
Comment #4
gambryThis is the first shape. I got it working locally, and it didn't require a lot of code... which is good.
It still requires tests, so uploading the patch just to show progress and give a workaround to whoever needs to move to V2 entities push soon.
Also: currently the
google/cloud-dialogflowpackage is on `required-dev` and `suggested`. In this way if you don't need the entities push handler you can avoid downloading shit-ton of google code.Comment #5
gambryTesting this is even worse. Google library uses a lot of static methods, which don't play nice with Prophecy.
I'll give it another go tomorrow.
Comment #6
larowlanThis is looking nice!
Comment #7
gambryUpdated handler and initial tests.
Tests only covers the
saveConfiguration()for now. Testing google EntityTypeClient has been a bit of a pain, but finally I've got there.We can improve tests later on, when we have more feedback from users using V2 entities push.
Of this work the only thing I'm not 100% sure are the two
finally {$client->close()}. I wanted to close the connection to save resources, but if$clientwill be called again after being closed gRPC library doesn't fail nice and instead segfaults brillianty leaving users without a clue. However currently they are harmless so I'm happy with having those in.Comment #8
gambryOoops. Empty patch. Re-uploading.
Comment #9
gambryFixing coding standard issues + require-dev chatbot_api in order to be available during tests.
Comment #10
larowlanfwiw I think you have to commit these to HEAD first in order for CI to work properly, that and a combination of entries in test_dependencies in info.yml (in my experience)
Looks solid - nice work!
Comment #11
gambry@larowlan this is definitely true for
test_dependenciesin info.yml (documentation mentions this) but the docs are unclear for require-dev in composer.json alternative.So I tried and apparently it works if changes to composer.json are in the patch, see the console output extract below:
and indeed tests against #9 do have ApiAiPushHandlerTest::testSaveConfiguration()
So, all looks good!
Comment #13
gambryThis is now merged in. I'll update the documentation today, and get a RC release out.
Setting NW to track the documentation task.
Comment #15
gambryDone with some documentation in the README.txt file. I'll point to this in the module description page.
Thanks!