Skip to content
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

Documentai Processor #6083

Merged
merged 8 commits into from
Jun 14, 2022
Merged

Conversation

slevenick
Copy link
Contributor

@slevenick slevenick commented May 26, 2022

Adds DocumentAI Processor resource and processor default version FGR

If this PR is for Terraform, I acknowledge that I have:

  • Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
  • Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
  • Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
  • Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
  • Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

google_documentai_processor
google_documentai_processor_default_version

@modular-magician
Copy link
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • new_resource

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 7 files changed, 822 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 7 files changed, 822 insertions(+), 2 deletions(-))
TF Validator: Diff ( 4 files changed, 100 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 213 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2030
Passed tests 1799
Skipped tests: 226
Failed tests: 5

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccFirebaserulesRelease_BasicRelease|TestAccDocumentAIProcessor_documentaiProcessorWithDefaultExample|TestAccDocumentAIProcessor_documentaiProcessorExample|TestAccBillingSubaccount_basic|TestAccBillingSubaccount_renameOnDestroy

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[view]

Tests failed during RECORDING mode:
TestAccDocumentAIProcessor_documentaiProcessorExample[view]
TestAccBillingSubaccount_renameOnDestroy[view]
TestAccBillingSubaccount_basic[view]
TestAccDocumentAIProcessor_documentaiProcessorWithDefaultExample[view]

Please fix these to complete your PR
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

Oops! It looks like you're using an unknown release-note type in your changelog entries:

  • new_resource

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 7 files changed, 619 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 7 files changed, 619 insertions(+), 2 deletions(-))
TF Validator: Diff ( 4 files changed, 90 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 106 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2031
Passed tests 1781
Skipped tests: 226
Failed tests: 24

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccFirebaserulesRelease_BasicRelease|TestAccContainerNodePool_withWorkloadIdentityConfig|TestAccDocumentAIProcessor_documentaiProcessorExample|TestAccContainerCluster_withDatabaseEncryption|TestAccContainerCluster_withSandboxConfig|TestAccContainerCluster_nodeAutoprovisioningDefaultsImageType|TestAccContainerCluster_withReleaseChannelEnabledDefaultVersion|TestAccContainerCluster_nodeAutoprovisioningDefaultsMinCpuPlatform|TestAccContainerCluster_withNodePoolUpdateVersion|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_errorAutopilotLocation|TestAccContainerCluster_withWorkloadIdentityConfig|TestAccContainerNodePool_withLinuxNodeConfig|TestAccContainerNodePool_withKubeletConfig|TestAccContainerCluster_withAutopilot|TestAccContainerCluster_withNotificationConfig|TestAccContainerCluster_withAddons|TestAccContainerNodePool_withSandboxConfig|TestAccContainerCluster_withWorkloadMetadataConfig|TestAccContainerCluster_nodeAutoprovisioningDefaults|TestAccContainerCluster_withBootDiskKmsKey|TestAccContainerCluster_updateVersion|TestAccContainerCluster_withVersion|TestAccContainerClusterDatasource_regional

@modular-magician
Copy link
Collaborator

The provider crashed while running the VCR tests in RECORDING mode
Please fix it to complete your PR
View the build log

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 7 files changed, 619 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 7 files changed, 619 insertions(+), 2 deletions(-))
TF Validator: Diff ( 4 files changed, 90 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 4 files changed, 106 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2033
Passed tests 1803
Skipped tests: 226
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccDocumentAIProcessor_documentaiProcessorExample|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withNotificationConfig|TestAccContainerCluster_withAddons

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccDocumentAIProcessor_documentaiProcessorExample[view]

Tests failed during RECORDING mode:
TestAccContainerCluster_withAddons[view]
TestAccContainerCluster_withConfidentialNodes[view]
TestAccContainerCluster_withNotificationConfig[view]

Please fix these to complete your PR
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 10 files changed, 952 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 10 files changed, 952 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 163 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 217 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2048
Passed tests 1818
Skipped tests: 226
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccFirebaserulesRelease_BasicRelease|TestAccDocumentAIProcessorDefaultVersion_documentaiDefaultVersionExample|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withAddons

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[view]
TestAccDocumentAIProcessorDefaultVersion_documentaiDefaultVersionExample[view]

Tests failed during RECORDING mode:
TestAccContainerCluster_withAddons[view]
TestAccContainerCluster_withConfidentialNodes[view]

Please fix these to complete your PR
View the build log or the debug log for each test

@slevenick slevenick changed the title Documentai Documentai Processor Jun 9, 2022
@slevenick slevenick requested review from a team and melinath and removed request for a team June 9, 2022 17:27
Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like mmv1/templates/terraform/post_create/documentai_processor.go.erb was added but is no longer used - could it be removed?

Relatedly - why does being able to directly set the field on the resource end up not working?

@slevenick
Copy link
Contributor Author

It looks like mmv1/templates/terraform/post_create/documentai_processor.go.erb was added but is no longer used - could it be removed?

Relatedly - why does being able to directly set the field on the resource end up not working?

Ah good call, that's from a previous impementation.

Setting the field during create isn't allowed by the API, but the bigger deal is that I would expect in the future for this to be able to reference user-created processor versions, which are sub-resources of this processor resource. So it will create a circular dependency between processor -> processorVersion -> defaultProcessorVersion

@slevenick slevenick requested a review from melinath June 13, 2022 21:59
@modular-magician
Copy link
Collaborator

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

Diff report:

Terraform GA: Diff ( 10 files changed, 952 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 10 files changed, 952 insertions(+), 2 deletions(-))
TF Validator: Diff ( 5 files changed, 163 insertions(+), 3 deletions(-))
TF OiCS: Diff ( 8 files changed, 217 insertions(+))

Copy link
Member

@melinath melinath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b/209050020

Gotcha - so the issue is that if a user tries to create a processor and a version and also use that version as the default version all in a single apply, Terraform wouldn't be able to process the dependency chain - but if the default version is set by a separate resource, that breaks the circular dependency and makes it work intuitively.

LGTM. Is there a precedent for this setup?

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2050
Passed tests 1821
Skipped tests: 226
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withAddons

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[view]

Tests failed during RECORDING mode:
TestAccContainerCluster_withAddons[view]
TestAccContainerCluster_withConfidentialNodes[view]

Please fix these to complete your PR
View the build log or the debug log for each test

@slevenick
Copy link
Contributor Author

Heres an example of a similar issue and the fix was adding a new FGR: hashicorp/terraform-provider-google#2686

Yeah, the issue is that we need to split out the defaultVersion to allow Terraform to build the dependency graph successfully

@modular-magician
Copy link
Collaborator

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

Diff report:

TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2052
Passed tests 1823
Skipped tests: 226
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests TestAccSqlUser_mysqlDisabled|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withAddons

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccSqlUser_mysqlDisabled[view]

Tests failed during RECORDING mode:
TestAccContainerCluster_withAddons[view]
TestAccContainerCluster_withConfidentialNodes[view]

Please fix these to complete your PR
View the build log or the debug log for each test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants