Skip to content

locking: make API requests relative to repository, not root#1818

Merged
ttaylorr merged 9 commits intomasterfrom
locking-endpoint
Jan 5, 2017
Merged

locking: make API requests relative to repository, not root#1818
ttaylorr merged 9 commits intomasterfrom
locking-endpoint

Conversation

@ttaylorr
Copy link
Contributor

This pull-request brings in some API changes which fixes making locking API requests at the root of the remote, instead of relative to the current repository.

It is currently based off of master, and should wait until #1815, #1816 & #1817 have been merged first. Once those three are merged into master, I'll rebase this PR off of master, and then merge. After this is merged into master, I think the changes should be merged back into api-master. Another approach we could take is to target this at api-master, to simplify some of the merging back and forth. I think it would be preferable to merge this into master first, if we cut a release off of master before api-master has been finished.

Here's a before and after:

--- /dev/fd/11  2016-12-30 12:44:03.000000000 -0700
+++ /dev/fd/12  2016-12-30 12:44:03.000000000 -0700
@@ -1 +1 @@
-https://github.com/locks
+https://github.com/owner/repo.git/info/lfs/locks

And a summary of what changed:

  1. 9c11250...043ee8b: don't use func (*net/url.URL) ResolveReference), allow for relative path joining.
  2. eb53d39...2e397a9: add an Accept and Content-Type header to all API requests made using the *api.Client type.
  3. d6df15d: ensure that /repo.git/info/lfs/locks is a GET request
  4. 51c35df...caf632f: some prep-work to make sure the {assert,refute}_server_lock helpers have the information they need to make requests at the correct location.
  5. ed34d39: update the test server to respond to locking requests at the correct URL 🎉

Before merging, I'd like to teach the gitserver how to store a per-repo cache of locks, instead of maintaining a global one.


/cc @git-lfs/core

@ttaylorr ttaylorr added this to the v2.0.0 milestone Dec 30, 2016
@ttaylorr ttaylorr mentioned this pull request Dec 30, 2016
5 tasks
Copy link
Contributor

@sinbad sinbad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants