backend/http: Return existing lock info from backend on conflict#38144
backend/http: Return existing lock info from backend on conflict#38144SarahFrench merged 3 commits intohashicorp:mainfrom
Conversation
|
|
nicholasngai
changed the title
nicholasngai
changed the title
nicholasngai
changed the title
|
Thanks for this submission! Per the Contributing.md, the maintainers as a matter of policy are generally not accepting changes to backends, however I will raise this one in triage as it seems like a fairly straightforward fix. Thanks! |
|
Thanks for the info! I actually am not sure I understand—does this mean that the maintainers would rather re-create this change themselves rather than merge in this PR? |
|
@nicholasngai No, there is no problem accepting the contribution, it is more that the backends are basically frozen in terms of changes. However, before I say anything definitive let me raise this in triage (next week) and get back to you. |
|
Sounds good! Thanks! |
… backend. Revert changes from #38144 to show that the test fails.
SarahFrench
left a comment
SarahFrench
left a comment
There was a problem hiding this comment.
Hi @nicholasngai, thanks for identifying this issue! It makes sense that the error should report the existing lock, like in the kubernetes backend. From surrounding code it looks like this was the intention by the original author.
I'd like there to be more tests covering this change. However, testing for backends isn't a well documented part of the codebase, due to reasons Craig has mentioned, and I don't want to lump you with a tough task. I've got a test prepared in #38192 and I can merge that, along with a small tweak to the changelog entry.
| @@ -0,0 +1,3 @@ | |||
| kind: BUG FIXES | |||
| body: 'backend: Return conflicting lock info from HTTP backend instead of the lock that failed to be taken' | |||
There was a problem hiding this comment.
| body: 'backend: Return conflicting lock info from HTTP backend instead of the lock that failed to be taken' | |
| body: 'backend/http: Return conflicting lock info from HTTP backend instead of the lock that failed to be taken' | |
| custom: | |
| Issue: "38144" |
There was a problem hiding this comment.
Sharing this here but it'll be addressed in the PR I mentioned. Usually in change log entries we include the issue number (or PR number if there's no issue) as this allows the final generated changelog to link here.
… backend. Revert changes from #38144 to show that the test fails.
|
Awesome, thanks everyone! 😄 |
* test: Update TestHTTPBackend so it can be created with pre-existing lock info inside it. This will be used for tests that want to assert the lock info returned with lock errors. * test: Add a test asserting data returned in lock errors from the http backend. * chore: Replace use of `io/ioutil` in http backend * chore: Update changelog entry to link to the original PR #38144 * test: Improve test code comments
3 participants
Hi folks! We return
inforight now from the HTTP state backend which is the info of the lock we are trying to acquire, whereas theLockErrorgenerally indicates that we want to return the lock that is currently being held (that we are conflicting with). This PR is a 1-liner change to do this instead.