-
Notifications
You must be signed in to change notification settings - Fork 2.9k
dialogflow: delete samples that are not in docs #2602
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
Conversation
…t are broken after the deletions
private static String TEST_KNOWLEDGE_BASE_ID = "MTA4MzE0ODY5NTczMTQzNzU2ODA"; | ||
private static String TEST_DOCUMENT_ID = "MTUwNjk0ODg1NTU4NzkzMDExMg"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be concerned about these constants and running testing jobs in parallel. (ie. j8 & j11)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this one. I used to have these tests dynamically create these resources, but the API has to do some ML training for an indeterminate time on the created resources which would create a flaky test occasionally. There are some more involved work-arounds to get around this, but would significantly complicate the tests.
So instead having this resource be static and used for "prediction" should handle any number of parallel requests and all of them should get an identical response and only a change to the Document we created should break this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor question - LGTM if you think it's ok.
* dialogflow: delete samples that are not in docs and fix the tests that are broken after the deletions * update README
* dialogflow: delete samples that are not in docs and fix the tests that are broken after the deletions * update README
fix the tests that are broken after the deletions
Update this PR: #2483
Fixes #2252