-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
adding location and doc schema #8371
Conversation
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @zli82016, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
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. Terraform GA: Diff ( 15 files changed, 2531 insertions(+), 2 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDocumentAIWarehouseLocation_documentAiWarehouseLocationExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaExample|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules |
|
/gcbrun |
Can you please add the copyright to the following files? Thanks. Files missing (or outdated) copyright: |
Hello, I have added the copyright to the mentioned files |
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. Terraform GA: Diff ( 15 files changed, 2531 insertions(+), 2 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. Please add acceptance tests which include these fields. |
You mentioned that |
Yes.
|
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDocumentAIWarehouseLocation_documentAiWarehouseLocationExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaExample|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeFirewallPolicyRule_multipleRules |
|
Two tests failed with the different error message TestAccDocumentAIWarehouseLocation_documentAiWarehouseLocationExample |
…sync, async in Location
…sync, async in Location
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. Terraform GA: Diff ( 16 files changed, 2672 insertions(+), 2 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. Please add acceptance tests which include these fields. |
Thanks so much for making the changes. Can you please add a separate test file in the tests folder to test the [PATCH operation]? (https://cloud.google.com/document-warehouse/docs/reference/rest/v1/projects.locations.documentSchemas/patch) Please check the documentation how to write the update tests. If you have any questions, please let me know. https://googlecloudplatform.github.io/magic-modules/develop/add-handwritten-test/#update-tests |
I have separated the tests into files, but I'm unsure of how to write the update tests properly. Also, considering that updating Document Schemas is not a customer requirement right now, can I add it later? |
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. Terraform GA: Diff ( 16 files changed, 3535 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 10 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaPropertyExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaFloatExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaIntegerExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaTextExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaTimestampExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaDatetimeExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaMapExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaEnumExample|TestAccDocumentAIWarehouseDocumentSchema_documentAiWarehouseDocumentSchemaPropertyEnumExample|TestAccContainerAwsNodePool_BetaBasicHandWritten |
Rerun these tests in REPLAYING mode to catch issues
|
This is an example to write updating test. https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/third_party/terraform/tests/resource_cloud_iot_device_update_test.go Also, do you know when updating Document Schemas will be a requirement for the users? |
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. Terraform GA: Diff ( 16 files changed, 3517 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerAwsNodePool_BetaBasicHandWritten |
|
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.
LGTM. Thanks.
* adding location and doc schema * added copyright * resolved required field, url_param_only field, description, autogen_async, async in Location * resolved required field, url_param_only field, description, autogen_async, async in Location * resolved hardcoded location, few fields in location, removed unnecessary enum values * resolved project_number, added fields in location example * DocumentSchema final changes * added timeouts in Location * adding missing fields from test report * added DocumentSchema tests * removed required field from processorType * changed dummy_values * removed fields under properties * added new line * added enum tests * separated tests into separate files * made Document Schema immutable
Adding a new service, Document AI Warehouse to Terraform. Added 2 resources, DocumentSchema and Location.
Location is not passing the acceptance tests, please provide some assistance
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
in the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)