dbt-labs / terraform-provider-dbtcloud

dbt Cloud Terraform Provider
https://registry.terraform.io/providers/dbt-labs/dbtcloud
MIT License
82 stars 19 forks source link

Feature/allow native GitHub repos #119

Closed b-per closed 1 year ago

b-per commented 1 year ago

Fixes #105

This PR adds the ability to connect to connect a repo using the native GitHub integration rather than the deploy_token one. This is similar to the approach already created for GitLab.

I have added a new test (that would work just for me though as the github_installation_id is currently hard code. The 2 tests are passing:

❯ /opt/homebrew/bin/go test -timeout 200s -run ^TestAccDbtCloudRepositoryResource$ github.com/gthesheep/terraform-provider-dbt-cloud/pkg/resources -v
=== RUN   TestAccDbtCloudRepositoryResource
--- PASS: TestAccDbtCloudRepositoryResource (37.85s)
PASS
ok      github.com/gthesheep/terraform-provider-dbt-cloud/pkg/resources 38.443s
(base) 

I have also added examples on how to create a dbt_cloud_repository resource and how someone could find their github_installation_id.

A key change I did as well was to move most of the repository fields to ForceNew: true, as I am not sure that the API lets us modify an existing repository. From the UI point of view, changing a repository requires in most of the cases disconnecting the existing repo and connecting a new one, similar to the ForceNew approach.

GtheSheep commented 1 year ago

/ok-to-test sha=c57959b

b-per commented 1 year ago

HI @GtheSheep !

Is there a way for me to help debugging the failing unit test? Clicking on "Details" doesn't show any detail for me.

GtheSheep commented 1 year ago

Hey @b-per! I think you can navigate to the run via the Actions part of the repo, lmk if not and I'll try and dig it out, not sure why it doesn't come through the details bit!

b-per commented 1 year ago

I can see difference tests on the Action tab but none fort this specific PR.

I just see this image

Drilling into image

And then image

b-per commented 1 year ago

For other actions, like this one, I can see the details.

(BTW those other ones are failing because dbt Cloud deprecated dbt-Core < 1.0 versions)

GtheSheep commented 1 year ago

@b-per - If you update your branch hopefully the tests/ dbt versions should be ok now 👌

b-per commented 1 year ago

Hi @GtheSheep ! I have pulled from main to retrigger the tests, and I can see the results now!

I think that there is a small issue with the acceptance tests, the env var DBT_CLOUD_ACCOUNT_ID doesn't seem to be set correctly.

GtheSheep commented 1 year ago

@b-per - Ahhh damn, this has made me realise why I had all of the ok-to-test stuff in place, forks don't have access to secrets in PRs 😬 I've changed to trigger on pull_request_target to try this out where it will require approval to run also.

In the meantime I think the only question is around those github_installation_ids?

b-per commented 1 year ago

Sure, what is your question about github_installation_id?

I have added instructions on how to retrieve it in the example for the dbt_cloud_repository but I don't know if there is anything else to add.

The github_installation_id will be unique for a given account, so it will need to be retrieved only once.

GtheSheep commented 1 year ago

Was just wondering if it was worth adding a call to retrieve it for the user, but could always do that later, it's not a sensitive piece of info, right?