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

Update to latest GRPC to make ent integ tests more reliable. #7117

Closed
wants to merge 3 commits into from

Conversation

ncabatoff
Copy link
Collaborator

I believe grpc/grpc-go#2818 is the cause of some of our intermittent test failures. The symptoms don't match exactly but they're close, and with this patch applied (along with my other fix) I wasn't able to provoke the "no handler for route" integ test failures. Note that this is apparently a fix for a 1.20 regression.

@ncabatoff ncabatoff requested a review from jefferai July 15, 2019 12:35
@jefferai
Copy link
Member

I think sdk uses grpc too, in which case the better approach would be to update sdk, then update vault main against new sdk. This should force vault main to use sdk's declared version.

@ncabatoff
Copy link
Collaborator Author

Done, not sure why api got updated too.

@ncabatoff
Copy link
Collaborator Author

Could you have another look please @jefferai?

@jefferai
Copy link
Member

Please update sdk first with the new version, then have Vault pull in the latest sdk.

@ncabatoff
Copy link
Collaborator Author

Please update sdk first with the new version, then have Vault pull in the latest sdk.

I had already done so. I've pulled in the latest changes from master, then tried doing the above a second time, but it was a no-op.

@ncabatoff ncabatoff added this to the 1.2 milestone Jul 23, 2019
@jefferai
Copy link
Member

You have to update SDK, then push a commit with that up to the repo, then update the main repo against the new SDK.

@ncabatoff
Copy link
Collaborator Author

Ah, that makes sense, I was just doing go get ./sdk.

@jefferai
Copy link
Member

Looks like you didn't go mod tidy for at least sdk. How did you get SDK in here -- what commit is that referring to, considering this PR also has the changes in sdk/go.mod?

@ncabatoff
Copy link
Collaborator Author

First commit in this branch was me doing it wrong, i.e.

cd sdk
go get google.golang.org/[email protected]
cd ..
go get ./sdk

Second commit was merge of master.

Third commit was result of

go get github.com/hashicorp/vault/sdk@update-grpc

Since I'm now confused and wanted a fresh start I created a new branch and will close this PR and work with that one instead (#7179).

@ncabatoff ncabatoff closed this Jul 24, 2019
@jefferai jefferai deleted the update-grpc branch July 24, 2019 15:32
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