-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add google_dialogflow_conversation_profile for (https://github.com/hashicorp/terraform-provider-google/issues/12466) #12859 #14324
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
base: main
Are you sure you want to change the base?
Conversation
…rce_dialogflow_conversation_profile_test.go
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @c2thorn, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
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.
some initial questions before running the build
drop_virtual_agent_messages = false | ||
} | ||
knowledge_base_query_source { | ||
knowledge_bases = ["projects/${google_project.agent_project.id}/locations/global/knowledgeBases/MTIxNTkzNjM1NzY3NjY2NjA2MDk"] |
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.
Is this a hardcoded source? Is it available to all, and what is it exactly
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.
Yes this is a hardcoded source. Knowledge Bases are not currently in the terraform google provider is there a better way to handle 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.
where did you get this value from?
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.
This is created manually
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.
@c2thorn Is there a change required here due to manually created knowledge base?
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.
These tests will need to run in an idempotent way in multiple of our test environments. Hardcoded resources need to be either universally available or we'll need to use bootstrap functions to ensure they exist everytime the test is run.
Do you have information on these specific values were created? Like do you own them specifically?
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.
Do you know if the value of the knowledge bases are actually verified by the API during the create request? If not, then we can maybe use placeholder values instead of a real value.
If the API does verify, then we may need to drop this field from tests.
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.
It seems the API requires a valid knowledge base (With document query source as well) in order for the conversation profile to be created.
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.
Hi @Spheny1, the knowledgeBaseQuerySource
and documentQuerySource
fields are related to the Agent Assist Article Suggestion and Agent Assist FAQ Assist features, which are superseded by Generative knowledge assist. So Google doesn't plan to add support for Agent Assist Article Suggestion
or Agent Assist FAQ Assist
in Terraform. Can you please drop the knowledgeBaseQuerySource
and documentQuerySource
fields in your PR? Thanks.
} | ||
`, context) | ||
} | ||
func testAccDialogflowConversationProfile_dialogflowAgentKnowledgebase2(context map[string]interface{}) string { |
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.
this is a very large config and can be hard to spot the difference between this and the first. Can you comment what's intended to change between the configs?
drop_virtual_agent_messages = false | ||
} | ||
document_query_source { | ||
documents = ["projects/ccai-shared-external/locations/global/knowledgeBases/smart_messaging_kb/documents/NzU1MDYzOTkxNzU0MjQwODE5Mg"] |
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.
please provide more info on these, and where they came from.
}) | ||
} | ||
|
||
// TODO make sure to flip agent back |
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.
Is this addressed?
clearing extraneous TODO
@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_dialogflow_conversation_profile" "primary" {
automated_agent_config {
session_ttl = # value needed
}
human_agent_assistant_config {
end_user_suggestion_config {
disable_high_latency_features_sync_delivery = # value needed
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
conversation_process_config {
recent_sentences_count = # value needed
}
disable_agent_query_logging = # value needed
enable_conversation_augmented_query = # value needed
enable_event_based_suggestion = # value needed
enable_query_suggestion_only = # value needed
enable_query_suggestion_when_no_answer = # value needed
query_config {
confidence_threshold = # value needed
context_filter_settings {
drop_handoff_messages = # value needed
drop_ivr_messages = # value needed
drop_virtual_agent_messages = # value needed
}
dialogflow_query_source {
agent = # value needed
human_agent_side_config {
agent = # value needed
}
}
document_query_source {
documents = # value needed
}
knowledge_base_query_source {
knowledge_bases = # value needed
}
max_results = # value needed
sections {
section_types = # value needed
}
}
suggestion_feature {
type = # value needed
}
suggestion_trigger_settings {
no_small_talk = # value needed
only_end_user = # value needed
}
}
generators = # value needed
group_suggestion_responses = # value needed
}
human_agent_suggestion_config {
disable_high_latency_features_sync_delivery = # value needed
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
}
conversation_process_config {
recent_sentences_count = # value needed
}
disable_agent_query_logging = # value needed
enable_conversation_augmented_query = # value needed
enable_query_suggestion_only = # value needed
enable_query_suggestion_when_no_answer = # value needed
query_config {
confidence_threshold = # value needed
dialogflow_query_source {
agent = # value needed
human_agent_side_config {
agent = # value needed
}
}
sections {
section_types = # value needed
}
}
suggestion_trigger_settings {
no_small_talk = # value needed
}
}
generators = # value needed
group_suggestion_responses = # value needed
}
notification_config {
message_format = # value needed
topic = # value needed
}
}
human_agent_handoff_config {
live_person_config {
account_number = # value needed
}
}
new_message_event_notification_config {
message_format = # value needed
topic = # value needed
}
notification_config {
message_format = # value needed
topic = # value needed
}
security_settings = # value needed
stt_config {
audio_encoding = # value needed
enable_word_info = # value needed
language_code = # value needed
model = # value needed
sample_rate_hertz = # value needed
speech_model_variant = # value needed
use_timeout_based_endpointing = # value needed
}
time_zone = # value needed
tts_config {
effects_profile_id = # value needed
pitch = # value needed
speaking_rate = # value needed
voice {
name = # value needed
ssml_gender = # value needed
}
volume_gain_db = # value needed
}
}
|
Tests analyticsTotal tests: 12 Click here to see the affected service packages
Found 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
resource "google_dialogflow_conversation_profile" "{{$.PrimaryResourceId}}" { | ||
display_name = "{{index $.Vars "profile_name"}}" | ||
location = "global" | ||
default_language_code = "en-US" |
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 this be language_code
instead?
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 this be
language_code
instead?
Yeah, it seems so.
@Spheny1 Could you please run the test locally by following https://googlecloudplatform.github.io/magic-modules/test/run-tests/? This is a large resource and the create
and update
tests can hit many errors. Including as many fields as possible in the create
tests and testing the create
test locally have worked well for me. The TF_LOG=DEBUG
flag is also super helpful. You can find how to use it in the doc I linked. Thanks!
depends_on = [google_project_iam_member.agent_create] | ||
} | ||
resouce "google_pubsub_topic" "topic" { |
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.
typo resouce
. Please fix. Thanks
} | ||
resource "google_dialogflow_agent" "agent" { | ||
project = "${google_project.agent_project.id}" |
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.
Found this error from the debug log: The requested URL <code>/v2/projects/projects/tf-test-dialogflow-o9bbm2mnzy/agent?alt=json</code> was not found on this server.
Problem is that projects
showed up twice.
projects/${google_project.agent_project.project_id}/...
should work according to https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/services/dialogflow/resource_dialogflow_intent_test.go#L207
drop_virtual_agent_messages = false | ||
} | ||
knowledge_base_query_source { | ||
knowledge_bases = ["projects/${google_project.agent_project.id}/locations/global/knowledgeBases/MTQ5NzI0MjQ0OTQwMTI5NTY2NzM"] |
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.
Saw error Error: Error creating ConversationProfile: googleapi: Error 400: Invalid resource name: 'projects/projects/tf-test-dialogflow-0kvg4kbd4x/locations/global/knowledgeBases/MTQ5NzI0MjQ0OTQwMTI5NTY2NzM'. Valid resource patterns are: 'projects/*/knowledgeBases/*'...
in the debug log. projects
appeared twice in the URL.
projects/${google_project.agent_project.project_id}/...
should work according to https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/services/dialogflow/resource_dialogflow_intent_test.go#L207
@Spheny1 Can you please add the missing fields mentioned in #14324 (comment) to your tests? Thanks! |
Removing unsupported query sources.
Working on passing tests.
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_dialogflow_conversation_profile" "primary" {
human_agent_assistant_config {
end_user_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
query_config {
document_query_source {
documents = # value needed
}
knowledge_base_query_source {
knowledge_bases = # value needed
}
}
}
generators = # value needed
}
human_agent_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
}
generators = # value needed
}
}
stt_config {
audio_encoding = # value needed
}
time_zone = # value needed
}
|
Tests analyticsTotal tests: 10 Click here to see the affected service packages
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Hi @Spheny1, could you please take a look at these two comments: |
Fixing conversation profile example.
@ArtoriaRen Im running into troubles getting the tests to complete without using my already standing project. Agent's need their own project due to the limitation of one Dialogflow ES agent per project and I am following a similar pattern to what already exists in entity or fulfillment resources but when creating the resources I am getting HTML 404 errors instead of any other meaningful errors. This seems to happen after a post request for the Pubsub topic. Pasting logs in this comment. |
Hi @Spheny1, in the log file you uploaded, I saw
I suspect this is because you are using |
@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_dialogflow_conversation_profile" "primary" {
human_agent_assistant_config {
end_user_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
query_config {
document_query_source {
documents = # value needed
}
knowledge_base_query_source {
knowledge_bases = # value needed
}
}
}
generators = # value needed
}
human_agent_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
}
generators = # value needed
}
}
stt_config {
audio_encoding = # value needed
}
time_zone = # value needed
}
|
Tests analyticsTotal tests: 10 Click here to see the affected service packages
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_dialogflow_conversation_profile" "primary" {
human_agent_assistant_config {
end_user_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
query_config {
document_query_source {
documents = # value needed
}
knowledge_base_query_source {
knowledge_bases = # value needed
}
}
}
generators = # value needed
}
human_agent_suggestion_config {
feature_configs {
conversation_model_config {
baseline_model_version = # value needed
model = # value needed
}
}
generators = # value needed
}
}
stt_config {
audio_encoding = # value needed
}
time_zone = # value needed
}
|
Tests analyticsTotal tests: 10 Click here to see the affected service packages
Found 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
🟢 Tests passed during RECORDING mode: 🔴 Tests failed when rerunning REPLAYING mode: Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
Add support for Dialogflow ConversationProfiles.
Release Note Template for Downstream PRs (will be copied)