[Feature Request] archive repo on destroy #432
Conversation
ghost
added
size/L
Type: Documentation
anGie44
added
acceptance-test/TestAccGithubRepository_
and removed
acceptance-test/TestAccGithubRepository_archive
labels
There was a problem hiding this comment.
Indented changes look good here. A bit of friction with acceptance tests though:
=== CONT TestAccGithubRepository_archiveUpdate
--- FAIL: TestAccGithubRepository_archiveUpdate (4.24s)
testing.go:654: Step 0 error: errors during apply:
Error: PATCH https://api.github.com/repos/terraformtesting/tf-acc-test-04yb6rh4g0: 404 Not Found []
on /var/folders/t1/p9cyhv6s2wg4g8s_x6zkpz_m0000gn/T/tf-test406284962/main.tf line 2:
(source code not available)
FAIL
FAIL github.com/terraform-providers/terraform-provider-github/github 4.491s
☝️ saw a few other tests fail as well when running locally, but it may be general acceptance test flakiness rather than what this PR proposes. I'll queue more time to investigate.
github/resource_github_repository.go
Outdated
| @@ -22,6 +22,7 @@ func resourceGithubRepository() *schema.Resource { | |||
| Importer: &schema.ResourceImporter{ | |||
| State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { | |||
| d.Set("auto_init", false) | |||
| d.Set("archive_on_destroy", false) | |||
There was a problem hiding this comment.
If a repository is imported with configuration that specifies archive_on_destroy to be true, will terraform perform the import and then update the resource's preference in some subsequent step? That seems like the probable flow, but wanted to ask in case you can confirm this behaviour.
There was a problem hiding this comment.
ah yes another plan and apply will be necessary after import as the API doesn't return this info at Read time; i'm now wondering what's a better way to approach this though.. set the archive_on_destroy to false here at import time, or don't explicitly set a value here? in either case there will be a non-empty plan after import
There was a problem hiding this comment.
ah or we could import by repo_name:archive_on_destroy instead to avoid the non-empty plan🤔
There was a problem hiding this comment.
@anGie44 I think the value this option provides outweighs the annoyance of a plan / apply step after an import.
Perhaps we can add a log statement during the import to explain this side effect rather than modifying the import ID.
|
Looks like recent updates to the branch protection plumbing has added conflicts to this PR. I've got this queued to address this week, but welcome anyone else jumping in to remove conflicts. |
jcudit
force-pushed
the
ap_425_archive_repo_on_destroy
branch
from
0878429 to
9320227
Compare
archive_on_destroyto thegithub_repositoryschema per [Feature Request] Option to archive repository on destroy #425