Dialogflow Intent Resource #3285

c2thorn · 2020-03-23T15:42:46Z

Fixes hashicorp/terraform-provider-google#5512

Release Note Template for Downstream PRs (will be copied)

`google_dialogflow_intent`

modular-magician · 2020-03-23T15:48:16Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 2140 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 2140 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 524 insertions(+))
TF OiCS: Diff ( 8 files changed, 262 insertions(+))

modular-magician · 2020-03-30T16:31:50Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 4 files changed, 1014 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 4 files changed, 1014 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 154 insertions(+))

modular-magician · 2020-03-30T16:48:38Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 5 files changed, 1026 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 1026 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 154 insertions(+))

chrisst

I have questions around the 'name' and the import flow, the rest is minor stuff.

chrisst · 2020-03-31T20:20:24Z

third_party/terraform/tests/resource_dialogflow_intent_test.go.erb

+		project = google_project_service.agent_project.project
+		role    = "roles/dialogflow.admin"
+		member  = "serviceAccount:service-${google_project.agent_project.number}@gcp-sa-dialogflow.iam.gserviceaccount.com"
+		depends_on = [google_project_service.agent_project]


This isn't needed because when you interpolate a value anywhere in this resource from an upstream resource (eg project = google_project_service.agent_project.project) the dependency is added implicitly.

chrisst · 2020-03-31T20:21:11Z

third_party/terraform/tests/resource_dialogflow_intent_test.go.erb

+	  }
+
+	resource "google_dialogflow_agent" "agent" {
+		project = "%s"


use interpolation here instead of string replacement

products/dialogflow/terraform.yaml

chrisst · 2020-03-31T20:29:20Z

products/dialogflow/api.yaml

+    name: 'Intent'
+    base_url: "projects/{{project}}/agent/intents/"
+    self_link: "{{name}}"


I'm confused what the value of 'name' is. I assumed based on the import format of "projects/{{project}}/agent/intents/{{name}}" that name would just be a uuid or some such unique identifier. But if that is the case then I don't think the self_link is https://dialogflow.googleapis.com/v2/{{a unique id}}

fyi I think by default self_link is {{base_url}}/{{name}

Whoops, that import format line was outdated and ignored because of the custom import. name here returns from the API in the format of projects/<Project ID>/agent/intents/<Intent ID> which is the same as the self_link. The API doesn't have a field for just <Intent ID>.

chrisst · 2020-03-31T20:31:48Z

products/dialogflow/api.yaml

+        description: |
+          Indicates whether webhooks are enabled for the intent.
+          * WEBHOOK_STATE_UNSPECIFIED: Webhook is disabled in the agent and in the intent.


Usually protobufs require that there is a default for all enums which is "blah_blah_unspecified". This is to guard against bugs where somebody forgot to actually set the value for the enum and is very rarely an acceptable value for the enum. If the API does not accept the unspecified value we usually strip it from the values we present to the user.

chrisst · 2020-03-31T20:35:26Z

products/dialogflow/api.yaml

+        description: |
+         The collection of event names that trigger the intent. If the collection of input contexts is not empty, all of
+         the contexts must be present in the active user session for an event to trigger this intent.


Is this completely unstructured or are there only certain events that the API will accept? It might help to link to a page that describes what 'events' are in this context.

chrisst · 2020-03-31T20:36:19Z

products/dialogflow/api.yaml

+            Represents different platforms that a rich message can be intended for.
+          values:
+            - :PLATFORM_UNSPECIFIED


same comment as unspecified enums above.

chrisst · 2020-03-31T20:47:21Z

templates/terraform/custom_import/self_link_as_name_set_project.go.erb

+			)
+	}
+
+	d.Set("project", stringParts[1])


Usually you only need to set the id of a resource and the subsequent get should be able to read the rest. If the 'name' is the equivalent of the self_link then you should be able to just set the id to the name without needing to parse the project from the string.

I believe we are overwriting the part that would normally parse the project. We need to use a custom import like index_self_link_as_name_set_project.go.erb, except for just the project field

modular-magician · 2020-04-03T18:35:16Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 6 files changed, 1194 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1194 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 154 insertions(+))

modular-magician · 2020-04-06T23:40:50Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 6 files changed, 1194 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 1194 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 154 insertions(+))

* Dialogflow Intent Resource * Remove larger fields for simpler reviews * dialogflow sidebar * Review changes * Update id formatting for docs

c2thorn added the upstream Requires an upstream API change/fix label Mar 23, 2020

googlebot added the cla: yes label Mar 23, 2020

c2thorn removed the upstream Requires an upstream API change/fix label Mar 25, 2020

c2thorn requested a review from chrisst March 30, 2020 17:00

chrisst reviewed Mar 31, 2020

View reviewed changes

c2thorn added 4 commits April 3, 2020 08:18

Dialogflow Intent Resource

70c3ff0

Remove larger fields for simpler reviews

e457de2

dialogflow sidebar

cfa55ff

Review changes

8ad6abe

c2thorn force-pushed the dialogflow branch from 05ec891 to 8ad6abe Compare April 3, 2020 18:30

c2thorn requested a review from chrisst April 3, 2020 18:59

Update id formatting for docs

6e9f864

chrisst approved these changes Apr 6, 2020

View reviewed changes

c2thorn merged commit bbaa9fd into GoogleCloudPlatform:master Apr 6, 2020

c2thorn deleted the dialogflow branch April 6, 2020 23:51

This was referenced Apr 6, 2020

Dialogflow Intent Resource GoogleCloudPlatform/terraform-google-conversion#411

Merged

Dialogflow Intent Resource hashicorp/terraform-provider-google#6061

Merged

Dialogflow Intent Resource hashicorp/terraform-provider-google-beta#1936

Merged

c2thorn mentioned this pull request Apr 9, 2020

DialogueFlow fulfillment support hashicorp/terraform-provider-google#6086

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dialogflow Intent Resource #3285

Dialogflow Intent Resource #3285

c2thorn commented Mar 23, 2020 •

edited

Loading

modular-magician commented Mar 23, 2020

modular-magician commented Mar 30, 2020

modular-magician commented Mar 30, 2020

chrisst left a comment

chrisst Mar 31, 2020

chrisst Mar 31, 2020

chrisst Mar 31, 2020

c2thorn Apr 3, 2020

chrisst Mar 31, 2020

chrisst Mar 31, 2020

chrisst Mar 31, 2020

chrisst Mar 31, 2020

c2thorn Apr 3, 2020 •

edited

Loading

modular-magician commented Apr 3, 2020

modular-magician commented Apr 6, 2020

Dialogflow Intent Resource #3285

Dialogflow Intent Resource #3285

Conversation

c2thorn commented Mar 23, 2020 • edited Loading

modular-magician commented Mar 23, 2020

Diff report:

modular-magician commented Mar 30, 2020

Diff report:

modular-magician commented Mar 30, 2020

Diff report:

chrisst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c2thorn Apr 3, 2020 • edited Loading

Choose a reason for hiding this comment

modular-magician commented Apr 3, 2020

Diff report:

modular-magician commented Apr 6, 2020

Diff report:

c2thorn commented Mar 23, 2020 •

edited

Loading

c2thorn Apr 3, 2020 •

edited

Loading