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

Go: improve transaction API #606

Merged
merged 3 commits into from
Jul 3, 2017
Merged

Go: improve transaction API #606

merged 3 commits into from
Jul 3, 2017

Conversation

djs55
Copy link
Collaborator

@djs55 djs55 commented Jul 2, 2017

Don't invite clients to name the temporary branch: they'll accidentally use non-unique strings which will cause problems when 2 Goroutines try to share the same branch name

Minor fixes:

  • fix one transaction leak
  • fix one clunk error

djs55 added 3 commits July 2, 2017 14:07
When updating the field values in the database, previously the code
would `return` if there was no work to do, leaking a branch in the
database. This patch guarantees to call `Commit` even if there is no
work to do.

Signed-off-by: David Scott <dave.scott@docker.com>
Previously `client.Remove` would call `clunk` after the `remove` and was
guaranteed to receive a failure from the server. This patch only clunks
the fid on the code paths before `remove` is called.

Signed-off-by: David Scott <dave.scott@docker.com>
Previously every call to NewTransaction needed to give a different
name for the temp branch since the branch namespace is shared. Clients
commonly used a constant string which would lead to concurrency failures
if the same function was called from 2 goroutines.

This patch removes the temporary branch name from the API and generates
a fresh one from an atomically-incremented counter. This still assumes
that only one client process can use this API at a time, but this is
better than assuming that only one Goroutine can use this at a time.

Signed-off-by: David Scott <dave.scott@docker.com>
@talex5 talex5 merged commit a8d0391 into moby:master Jul 3, 2017
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 21, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
samoht added a commit to samoht/opam-repository that referenced this pull request Nov 23, 2017
CHANGES:

- all: update to latest version of alcotest, conduit, session, ocaml-github,
  ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55)

- github: make `User.t` abstract (moby/datakit#594, @samoht)
- github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht)
- github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht)
- github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht)

- github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht)

- go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel)
- go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55)
- go-client: improve transaction API (moby/datakit#606, @djs55)
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.

None yet

2 participants