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

Missing GitHub repository data source attributes #778

Conversation

cnuss
Copy link
Contributor

@cnuss cnuss commented May 6, 2021

Greetings!

I've found in one of my projects that the Data Source for GitHub repositories continually needs to be refreshed, for example:

Terraform will perform the following actions:

  # module.bootstrap.module.github_config_files_public_websites["cdn"].data.github_repository.repository will be read during apply
  # (config refers to values not yet known)
 <= data "github_repository" "repository"  {
      ~ allow_merge_commit = true -> (known after apply)
      ~ allow_rebase_merge = true -> (known after apply)
      ~ allow_squash_merge = true -> (known after apply)
      ~ archived           = false -> (known after apply)
      ~ default_branch     = "main" -> (known after apply)
      + description        = (known after apply)
      ~ git_clone_url      = "git://github.com/futz-dev/cdn-web-cdn.git" -> (known after apply)
      ~ has_downloads      = false -> (known after apply)
      ~ has_issues         = false -> (known after apply)
      + has_projects       = (known after apply)
      ~ has_wiki           = false -> (known after apply)
      + homepage_url       = (known after apply)
      ~ html_url           = "https://github.com/futz-dev/cdn-web-cdn" -> (known after apply)
      ~ http_clone_url     = "https://github.com/futz-dev/cdn-web-cdn.git" -> (known after apply)
      ~ id                 = "cdn-web-cdn" -> (known after apply)
      - name               = "cdn-web-cdn" -> null
      ~ node_id            = "MDEwOlJlcG9zaXRvcnkzNjQ1MjI2MDI=" -> (known after apply)
      + pages              = (known after apply)
      ~ private            = true -> (known after apply)
      ~ repo_id            = 364522602 -> (known after apply)
      ~ ssh_clone_url      = "git@github.com:futz-dev/cdn-web-cdn.git" -> (known after apply)
      ~ svn_url            = "https://github.com/futz-dev/cdn-web-cdn" -> (known after apply)
      ~ topics             = [] -> (known after apply)
      ~ visibility         = "private" -> (known after apply)
        # (1 unchanged attribute hidden)
    }

As you can see in the output, the following attributes are causing a refresh to be made:

  • name
  • description
  • has_projects
  • homepage_url
  • pages

Upon diving into these I made the following fixes:

  • name: Should be name from the API, not the calculated name/full_name setting.
  • description is set to null when unset from the API
  • has_projects: Was missing from dataSourceGithubRepositoryRead(...)
  • homepage_url: Defaults to an empty string when unset from the API
  • pages: Should be initialized to an empty list instead of simply being excluded

I also wrote some tests around this, but let me know if I should add some more. Here's the final output of running TestAccGithubRepositoryDataSource locally:

-----------------------------------------------------
2021/05/06 12:21:59 [DEBUG] Resource state not found for node "data.github_repository.test", instance data.github_repository.test
2021/05/06 12:21:59 [DEBUG] ReferenceTransformer: "data.github_repository.test" references: []
2021/05/06 12:21:59 [DEBUG] provider has no plugin.Client
2021/05/06 12:21:59 [DEBUG] New state was assigned lineage "dca8899a-b369-56fd-a60a-8e0349654ccd"
2021/05/06 12:21:59 [INFO] terraform: building graph: GraphTypePlanDestroy
2021/05/06 12:21:59 [DEBUG] Starting graph walk: walkPlanDestroy
--- PASS: TestAccGithubRepositoryDataSource (20.80s)
    --- PASS: TestAccGithubRepositoryDataSource/anonymously_queries_a_repository_without_error (0.00s)
        --- SKIP: TestAccGithubRepositoryDataSource/anonymously_queries_a_repository_without_error/with_an_anonymous_account (0.00s)
    --- PASS: TestAccGithubRepositoryDataSource/raises_expected_errors_when_querying_for_a_repository (0.99s)
        --- SKIP: TestAccGithubRepositoryDataSource/raises_expected_errors_when_querying_for_a_repository/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryDataSource/raises_expected_errors_when_querying_for_a_repository/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryDataSource/raises_expected_errors_when_querying_for_a_repository/with_an_organization_account (0.99s)
    --- PASS: TestAccGithubRepositoryDataSource/queries_a_repository_with_pages_configured (10.92s)
        --- SKIP: TestAccGithubRepositoryDataSource/queries_a_repository_with_pages_configured/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryDataSource/queries_a_repository_with_pages_configured/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryDataSource/queries_a_repository_with_pages_configured/with_an_organization_account (10.92s)
    --- PASS: TestAccGithubRepositoryDataSource/checks_defaults_on_a_new_repository (8.89s)
        --- SKIP: TestAccGithubRepositoryDataSource/checks_defaults_on_a_new_repository/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryDataSource/checks_defaults_on_a_new_repository/with_an_organization_account (8.89s)
PASS
ok  	github.com/terraform-providers/terraform-provider-github/github	21.102s

Let me know what y'all think of this!

CC'ing prior authors on this data source for additional feedback:
@jcudit @anGie44 @patrickmarabeas @tracypholmes @ryndaniels @radeksimko @martinssipenko

@cnuss
Copy link
Contributor Author

cnuss commented May 11, 2021

@jcudit could you unlock the workflow run so i can make sure that checks pass?

@cnuss
Copy link
Contributor Author

cnuss commented May 14, 2021

@jcudit @anGie44 @patrickmarabeas @tracypholmes @ryndaniels @radeksimko @martinssipenko

checks pass, we good to move forward on this?

Copy link
Contributor

@jcudit jcudit left a comment

Choose a reason for hiding this comment

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

🙇🏾 thanks for these fixes!

@jcudit
Copy link
Contributor

jcudit commented May 22, 2021

Looks like another PR has introduced a vendoring conflict. Will merge this as-is and follow up with 3a71e80 to get CI passing.

@jcudit jcudit merged commit bae2233 into integrations:master May 22, 2021
@cnuss cnuss deleted the missing-github-repository-data-source-attributes branch May 22, 2021 02:35
jcudit pushed a commit that referenced this pull request Jun 16, 2021
* has_projects

* description default

* Other attributes

* Test on name
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
* has_projects

* description default

* Other attributes

* Test on name
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.

2 participants