aptly icon indicating copy to clipboard operation
aptly copied to clipboard

1125 api failures and race conditions with concurrent publish update operations

Open randombenj opened this issue 1 year ago • 1 comments

Fixes #1125

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

While implementing the ?_async=true task api, resources which must not be handled concurrently were protected by resource groups. However the non async resources could still be operated on concurrently by just doing two api requests at the same time.

This change fixes this behaviour by running sync operations in the new task framework and waiting internally before returning a result.

Checklist

  • [ ] unit-test added (if change is algorithm)
  • [x] functional test added/updated (if change is functional)
  • [ ] man page updated (if applicable)
  • [ ] bash completion updated (if applicable)
  • [ ] documentation updated
  • [ ] author name in AUTHORS

randombenj avatar Mar 09 '23 14:03 randombenj

You may be aware, but this pull request mixes concerns that seem completely unrelated to me:

  • Why we do need to deprecate calls to rand.Seed is not explained, and more importantly, I do not see how this is connected to solving API lock errors a.k.a. API race conditions. Why do we need this in here?
  • Why should support for multiple Go versions be dropped from the CI (, or generally, following the commit message)?
  • Why do we fix a CI linting issue here? Why is this problem seemingly only tracked with a FIXME comment and a link to our dependency's issues?
  • Why do we need to bump the Go version here?

Please excuse my rather rude demeanour, I think your changes should probably all be merged, but I see little reasoning as to why this should all be a single pull request.

r4co0n avatar Jul 07 '23 17:07 r4co0n