fix: remove repository topic from state if it doesnt exist in GitHub anymore #1918
Conversation
felixlut
changed the title
|
Thanks @felixlut your use case and approach definitely seem reasonable. I am tempted to make this a "breaking change" because it changes the current behavior of the provider - whereas previously a 404 was expected now you get an info log and passthrough. Let me chat with @kfcampbell about this and get his take on it. I like the change I just don't want to end up harming other's implementations that assume they will see a 404 and take action on that. Though, unlikely, I still want to do the due diligence. |
Thank you for your reply! Yes, I agree, this is actually a breaking change. I missed it while creating the PR, my apologies 😅 I think it shouldn't be too hard to add an argument in the resource for avoiding the breaking change if you want. Something along the lines of |
nickfloyd
added
the
Type: Breaking change
|
Hey! Any update on this? |
|
Hey @felixlut I'm still trying to gather up some details and see if there are any other breaking changes out there that we could get in. Hashicorp has a strong recommendation of not having more than one major version bump aper year that I want to be sensitive to and @kfcampbell is currently out until next week - I want to make sure he's had a moment to look over the potential of a major bump as well. Apologies for the delay on this - we'll get it in as soon as we have a better idea of what we can meaningfully ship with this change set. Thank you for the patience on this. |
|
@nickfloyd no issue, just wanted to know it wasn't completely forgotten! I've been running a fork of your provider since the opening of this PR, and want to get back to stable waters. On the positive side, this change has been tested for a bit 😅 |
|
@felixlut we'd love to publish a new major version in the near future (next couple months?) pending our team's capacity to do so. Thanks for your persistence here! |
|
I'm going to merge this PR into our |
* Add retryable transport for errors (#1704) * Add retryable transport for errors In order to address the issue #210 I have added 3 new parameters to the provider - retry_delay_ms - max_retries - retryable_errors In case max_retries is > 0 (defaults to zero) it will retry the request in case it is an error the retryable_errors defaults to [500, 502, 503, 504] It retries after the ms specified in retry_delay_ms (defaults to 1000) * Update documentation with new parameters for retry * Change default of max_retries to 3 * Fix typo in naming * Update github/transport_test.go * Add error check for data seek * Consolidate the default retriable errors on a function * Fix typo on the comments Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Update vendor * Fix merging conflicts * Update documentation with new parameters for retry * Change default of max_retries to 3 * Fix typo in naming * Add error check for data seek * Update github/transport_test.go * Consolidate the default retriable errors on a function * Fix typo on the comments Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Don't run go mod tidy on release (#1788) * Don't run go mod tidy on release * Be more specific about releases * Fix lint with APIMeta deprecation --------- Co-authored-by: Keegan Campbell <me@kfcampbell.com> Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com> * fix: remove repository topic from state if it doesnt exist in GitHub anymore (#1918) * remove repository topic if they cannot be found in GitHub anymore * Fix build error by bumping package version in offending file --------- Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Bump version to v6 (#2106) * Upgrade to Terraform Plugin SDK v2 (#1780) * go mod tidy -go=1.16 && go mod tidy -go=1.17 * Run go mod vendor * Attempt v2 upgrade * Plugin compiling * Fix some provider test errors * Fix test compilation error * ValidateFunc --> ValidateDiagFunc * Fix casing * Sprinkle toDiagFunc everywhere * More fixes for validation functions * State --> StateContext * %s --> %v when printing diags * ConfigureFunc --> ConfigureContextFunc * Checking results of d.Set, round one * Continue checking d.Set results * Check results of d.Set, round three * Checking d.Set results, round four * d.Set round five * In tests, export GITHUB_TEST_ORGANIZATION * Remove unnecessary MaxItems on computed value * Go build now works * Resolve linting errors * Apply diag.FromErr twice more * Pass key names into toDiagFunc helper * Construct cty.Path from strings * Tests now working * Update terraform-plugin-sdk version * Remove commented attribute setting in resource_github_team.go * Fix restrict pushes on github_branch_protection. Fix branch protection tests (#2045) * Update restrict pushes. Fix branch protection tests Change blocks_creations default to true. Fix broken build. * add state migration * rename push_restrictions to push_allowances * correct state migration issue * add docs clarification * update migration func args * fix test args * cleanup tests * Pass context.Background() in test * fix timestamp fields --------- Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Set group_id correctly (#2133) * Run go get -u github.com/golangci/golangci-lint * Separate github_team_members import from github_team as create_default_maintainers is not defined for members resource (#2126) Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Add missing variable definition for test case --------- Co-authored-by: Daniel França <github.t6297kgphp.dv@koderama.com> Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com> Co-authored-by: Felix Luthman <34520175+felixlut@users.noreply.github.com> Co-authored-by: georgekaz <1391828+georgekaz@users.noreply.github.com> Co-authored-by: Rich Young <richjyoung@users.noreply.github.com>
Before the change?
terraform planon a configuration including arepository_topicresource when the repository itself has been deleted outside of terraform, you get a404 Not Founderror.After the change?
terraform planon a configuration including arepository_topicresource when the repository itself has been deleted outside of terraform, terraform will simply remove the topics from its state. This makes it easier to deal with drifts. My use-case is that I userepository_topicas a standalone resource without a standalonerepositoryresource, and instead reads a datasource for the repo.Pull request checklist
Does this introduce a breaking change?