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

Use batch API for Download, fallback to legacy if unsupported #711

Merged
merged 2 commits into from Oct 21, 2015

Conversation

rubyist
Copy link
Contributor

@rubyist rubyist commented Oct 6, 2015

The smudge filters still used the direct legacy api via the Download() function. This PR provides a new Download() which uses the batch API and falls back to a DownloadLegacy() if the server does not support the batch API.

This fixes #708

The smudge filters still used the direct legacy api via the Download()
function. It should now go through the batch API and only use the legacy
API if the server doesn't support the batch operations.
@rubyist
Copy link
Contributor Author

rubyist commented Oct 6, 2015

I think there's still plenty to refactor around this code, but we might want to hold off on that until we completely remove support of the legacy API. Then we can just remove all the legacy cruft at once.

@sinbad
Copy link
Contributor

sinbad commented Oct 6, 2015

👍
Yeah it's slightly confusing at the moment because DownloadCheck() also uses the old API but then it's only ever called by the individualApiRoutine, so the use of the batch version & fallback is higher up. Download() is only ever used by the smudge filter now though so this does address the core issue for now, so I think it's still worth doing ahead of a more general tidy up.

@technoweenie
Copy link
Contributor

I updated this so that lfs.Download() only attempts batch transfers if lfs.batch is not false.

so I think it's still worth doing ahead of a more general tidy up.

We absolutely have to do this before starting on your SSH pet project :)

@technoweenie technoweenie mentioned this pull request Oct 20, 2015
20 tasks
@rubyist
Copy link
Contributor Author

rubyist commented Oct 20, 2015

👍 to your changes

technoweenie added a commit that referenced this pull request Oct 21, 2015
Use batch API for Download, fallback to legacy if unsupported
@technoweenie technoweenie merged commit f56cd54 into master Oct 21, 2015
@technoweenie technoweenie deleted the smudge-new-api branch October 21, 2015 16:26
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.

Client still depends on legacy API
3 participants