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

Upgrade to go-github version 29.0.3 #368

Closed
martinssipenko opened this issue Feb 28, 2020 · 5 comments
Closed

Upgrade to go-github version 29.0.3 #368

martinssipenko opened this issue Feb 28, 2020 · 5 comments

Comments

@martinssipenko
Copy link
Contributor

I wanted to create a pull request to upgrade go-github to version 29.0.3, however, I've stumbled across issues that I'd like to discuss prior moving forward with implementation.

The issue is that GitHub has deprecated some of the Team API endpoints (marked as Legacy here). After this change go-github has made a braking change in version 29.0.3, where they removed some methods by replacing them with others methods.

This terraform provider currently uses the deleted methods in resource_github_team.go (GetTeam, EditTeam, DeleteTeam) and resource_github_team_repository.go (AddTeamRepo, IsTeamRepo, RemoveTeamRepo).

This makes the upgrade hard because the provider code needs to be updated to use new methods of go-github, but the new methods require extra arguments. For example AddTeamRepo is now replaced by AddTeamRepoByID and AddTeamRepoBySlug. AddTeamRepoByID requires teamId (int) which we do have and orgID (int) which we don't have in state. The other method AddTeamRepoByID requires orgName (string) which we do have in provider config and slug (string) which we do have in state, but not as part of ID.

The situation is even trickier with resource_github_team_repository.go, for example AddTeamRepo func is now replaced by either AddTeamRepoByID or AddTeamRepoBySlug. The AddTeamRepoByID requires orgID (int) which we don't have in state and teamID (int) which is passed as argument into resource. The AddTeamRepoBySlug requires orgName (string), which we do have in provider config, and taem slug (string) which we don't have available.

I'm looking for a guidence to how to solve this issue. Should we increment the schema version and use StateUpgraders to get the missing data into state? Or should we rethink the interface for affected resources?

@benj-fletch
Copy link
Contributor

Hi @martinssipenko I was looking at this independently of you based on our discussion on #362.

I have implemented a method of retrieving the orgID (int) as a computed value to the provider. Does this address the issues you raise with existing state?

@martinssipenko
Copy link
Contributor Author

@benj-fletch yes, i think they do.

@arri-cc
Copy link

arri-cc commented Feb 28, 2020

@martinssipenko You beat me to it, I was looking into this last night to try to solve #305.

@martinssipenko
Copy link
Contributor Author

@arri-cc check out #369 , which will hopefully fix this issue and unblock few others.

@benj-fletch
Copy link
Contributor

I think this issue can now be closed following the merge of #369 ?

@jcudit jcudit closed this as completed Apr 1, 2020
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

No branches or pull requests

4 participants