Port to oauth2, fix #606 #900
Conversation
Currently when running `make updatedeps` from a branch, the dependency list from master ends up getting used. We tried to work around this in 35490f7, and got part way there, but here's what was happening: - record the current SHA - run `go get -f -u -v ./...` which ends up checking out master - master is checked out early in the `go get` process, which means all subsequent dependencies are resolved from master - re-checkout the recorded SHA - run tests This works in most cases, except when the branch being tested actually changes the list of dependencies in some way. Here we move away from letting `go get -v` walk through everything in `./...`, instead building our own list of dependencies with the help of `deplist`. We can then filter terraform packages out from the list, so they don't get touched, and safely update the rest. This should solve problems like those observed in #899 and #900. __Note__: had to add a feature to deplist to make this work properly; see phinze/deplist@016ef97 Working on getting it accepted upstream.
|
Alright now that #901 has landed, I think if you rebase on master then all should be well! |
|
Can one of you comment on my original question about the prompt? thanks |
Seems fine to me too, and you mention it in the docs. 👍 |
| "github.com/rasa/oauth2-fork-b3f9a68" | ||
|
|
||
| // oauth2 "github.com/rasa/oauth2-fork-b3f9a68/google" | ||
| "github.com/rasa/oauth2-fork-b3f9a68/google" |
There was a problem hiding this comment.
Can we verify that this fork is still necessary? I found hashicorp/packer@10dee1b but I can't seem to find any commentary on what exactly broke and what needs to be fixed upstream.
There was a problem hiding this comment.
I am not sure. I tried github.com/golang/oauth2/google and couldn't work out how to make the build systems pull in the dependency. When I saw packer using the other one I just copied that and it worked. I'd like to know the full story as well.
|
@phinze re: comment earlier about whether the fork is still necessary, this is upstream (copied from the Packer PR), but it won't build (updatedeps) and I'm not sure why. Any ideas? thanks |
|
@lamielle Oops I forgot to tag you. Better late than never! |
|
Thanks @sparkprime! Glad to see this PR! |
|
@phinze Thanks for the pointers, seems to be OK now. The same modifications could (should?) be made to Packer. |
|
Nice! Much cleaner. 🚿 |
Currently when running `make updatedeps` from a branch, the dependency list from master ends up getting used. We tried to work around this in 35490f7, and got part way there, but here's what was happening: - record the current SHA - run `go get -f -u -v ./...` which ends up checking out master - master is checked out early in the `go get` process, which means all subsequent dependencies are resolved from master - re-checkout the recorded SHA - run tests This works in most cases, except when the branch being tested actually changes the list of dependencies in some way. Here we move away from letting `go get -v` walk through everything in `./...`, instead building our own list of dependencies with the help of `deplist`. We can then filter terraform packages out from the list, so they don't get touched, and safely update the rest. This should solve problems like those observed in hashicorp#899 and hashicorp#900. __Note__: had to add a feature to deplist to make this work properly; see phinze/deplist@016ef97 Working on getting it accepted upstream.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Terraform equivalent of part 1 of hashicorp/packer#1679
One question: If the account_file is not given it should attempt to get tokens from the metadata server. However I cannot figure out how to avoid this prompt:
At the moment you have to explicitly specify the empty string in the config to avoid the prompt. In my opinion this is fine but is it what you want?