Azure DevOps - LFS sync - Poor implementation
Proposal
A customer is trying to use the repository sync on a couple of projects, while pushing them to Azure DevOps. It was noticed that the LFS sync is very poorly implemented.
Customer feedback:
- Default sync timeouts are way too low (20 read, 30 write), which are too low for very big files, which is what you would use LFS for
- Not enough logging, there is no way to see what's going on internally in the sync process
- When a request is unsuccessful, it only reports a ObjectUploadError, nothing else, no way to see what's going on, or what caused the error
- The sync job still reports as successful in GitLab, even though LFS failed, and it does not show the LFS error in the interface
- The sync does not use git-lfs but it's own implementation, which causes it not to use the .lfsconfig file and .gitconfig
The customer has investigated our codebase in order to understand why the sync was failing.
Additional notes from the customer regarding Azure DevOps sync:
Content-Length
GitLab always sets the Content-Length
header, even though the LFS server requests chunked encoding, causing request issues because the server thinks the body will be too big,
That can be fixed by changing https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/lfs/client.rb by remvoing the Content-Length
in the upload method, while adding the following:
# Only set Content-Length when we do not use chunked encoding
# @todo: also check value of Transfer-Encoding header to contain chunked
params[:headers]['Content-Length'] if !params[:headers].key?('Transfer-Encoding')
Batch action's auth user doesn't overwrite the credentials
Azure DevOps adds the basic auth user to the URL in the batch action, when that URL is given to HTTParty, it does not override the username/password from the basic auth given with: params[:basic_auth] = basic_auth unless authenticated
That can be fixed by doing this:
# If we did not get an Authorization header, set the basic auth
if !authenticated
params[:basic_auth] = basic_auth unless authenticated
# Remove the user from the href if it's in there, or httparty won't take params[:basic_auth]
parsed_href = URI::parse(upload_action['href'])
parsed_href.userinfo = ''
upload_action['href'] = parsed_href.to_s
end