-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(work pools): fix work pool base job template "inconsistent values" #249
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Copies the simpler way of decoding JSON that we use for Blocks.
The base_job_template in the plan can differ from the base_job_template in the work pool object retrieved from the API, specifically in the order of the fields. In these cases, the objects are functionally equivalent, but Terraform returns an error because it retrieved an "inconsistent result after apply." This change accounts for those differences by comparing the objects and checking for equality using the go-cmp package. If the objects are not equal, we will return an error diagnostic.
mitchnielsen
had a problem deploying
to
Acceptance Tests
August 23, 2024 02:10
— with
GitHub Actions
Failure
The BaseJobTemplate field is large due to the number of required fields. As a result, when there's a failure, printing the entire struct twice (once for expected, once for got) is very hard to parse visually - especially because Terraform logs them as JSON objects with quotes escaped and with everything on one "line." This package wraps go-cmp, which was used for comparison in the previous commit. It adds on the ability to return the differences as a string, which is much more readable in the output. As a contrived example, when the resource retrieved from the API has extra fields: ``` prefect_work_pool.example_work_pool Modifying... ╷ │ Error: Unexpected difference between base_job_template │ │ Expected the provided base_job_template to be equal to the one retrieved from the API, differences: { │ "prop-added":{"job_configuration": {} │ "prop-added":{"auto_deregister_task_definition": "{{ auto_deregister_task_definition }}",} │ "prop-added":{"cloudwatch_logs_options": "{{ cloudwatch_logs_options }}",} ```
Ensures that the base_job_template field is tested in the work pool tests. Note that we don't use the TestCheckResourceAttr check because it can't account for differences in the JSON object ordering.
mitchnielsen
temporarily deployed
to
Acceptance Tests
August 23, 2024 03:48
— with
GitHub Actions
Inactive
mitchnielsen
commented
Aug 23, 2024
This package allows comparing interfaces, so it's more flexible and requires less conversion between types. Since it's more flexible, also moves the function to a helper so we can use it to compare other objects in the provider in the future. Example of the output when the objects do not map: ``` === RUN TestAccResource_work_pool work_pool_test.go:58: Step 1/6 error: Check failed: Check 2/5 error: Found unexpected differences in work pool base_job_template: map[job_configuration]: map[command:{{ command }} env:{{ env }} labels:{{ labels }} name:{{ name }} stream_output:{{ stream_output }} working_dir:{{ working_dir }}] != <does not have key> ```
mitchnielsen
temporarily deployed
to
Acceptance Tests
August 23, 2024 15:32
— with
GitHub Actions
Inactive
parkedwards
reviewed
Aug 23, 2024
parkedwards
reviewed
Aug 23, 2024
// If the base job template from the retrieved work pool is equal | ||
// to the base job template from the plan (taking into account that the | ||
// fields could be in a different order), then we will use the plan's definition | ||
// of the base job template in the state. This avoids "inconsistent value" |
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.
smart
parkedwards
approved these changes
Aug 23, 2024
Co-authored-by: Edward Park <ed.sh.park@gmail.com>
mitchnielsen
temporarily deployed
to
Acceptance Tests
August 23, 2024 16:26
— with
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
base_job_template
to the Work Pool testsMore information on each change is available in the relevant commit message.
Closes #244