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

check if file was already downloaded #642

Closed
wants to merge 5 commits into from
Closed

check if file was already downloaded #642

wants to merge 5 commits into from

Conversation

andreiko
Copy link
Contributor

In --download mode skip downloading when file already exists in target directory

In --download mode skip downloading when file already exists in target directory
@travisbot
Copy link

This pull request passes (merged d40644d into 1749343).

@travisbot
Copy link

This pull request passes (merged 80a2ffc into 1749343).

@travisbot
Copy link

This pull request passes (merged e350965 into 1749343).

@mcdonc
Copy link
Contributor

mcdonc commented Sep 7, 2012

Could we tweak you into supplying a set of tests for this behavior (e.g. in tests/test_download.py)?

@carljm
Copy link
Contributor

carljm commented Sep 7, 2012

I'm also curious why this is better than just using pip's existing download cache feature (i.e. by setting the PIP_DOWNLOAD_CACHE env variable)? The existing download cache is smarter in that it doesn't assume that same filename == same file, it caches based on the full source URL. And it's a general feature of pip rather than specific to --download.

Closing for now as a duplicate of an existing feature, will re-open if there's a good rationale presented for why this is needed.

@carljm carljm closed this Sep 7, 2012
@alexandrul
Copy link

Well, placing some packages by hand in the destination folder would be useless, as they wouldn't have an entry in pip cache.

IMHO, pip cache is very useful for regular use, but this pull request would be very nice to have.

@carljm
Copy link
Contributor

carljm commented Sep 8, 2012

So currently if the destination file exists, --download prompts to ask if you want to override or skip, and if you choose to skip it doesn't download again. It seems to me that's reasonable default behavior; if we want to add something maybe it should be an optional additional command-line flag to pre-supply an answer to that question?

@andreiko
Copy link
Contributor Author

andreiko commented Sep 8, 2012

Actually, file is always being downloaded, and THEN it's asked if it should be replaced in the destination folder.

@andreiko
Copy link
Contributor Author

andreiko commented Sep 8, 2012

I wanted my CI to install packages inside virtualenv without any internet connection. Option of --download-cache doesn't cache requests to index. So I decided to collect all packages to a single folder and install with --no-index and --find-links. When adding new packages from updated requirements.txt file pip always wanted to re-download all packages. I changed this behavior for myself and thought it could be useful in upstream.

@carljm
Copy link
Contributor

carljm commented Sep 8, 2012

Yes, you're right about the current behavior. Ok, I'm convinced this is worth adding, so that brings us back to "tests needed."

@carljm carljm reopened this Sep 8, 2012
@andreiko
Copy link
Contributor Author

Added a test for this.

@carljm
Copy link
Contributor

carljm commented Sep 10, 2012

Merged, thanks!

@carljm carljm closed this Sep 10, 2012
@carljm
Copy link
Contributor

carljm commented Sep 10, 2012

Hmm, should have waited for the Travis tests to run before merging. Tests pass locally, but the new test fails on Travis. Which is odd, because the new test copies patterns from existing tests that are passing on Travis.

@andreiko
Copy link
Contributor Author

Oh, I should have run this on linux too. Test is fixed in my 15e76ce. Should I open another pull request?

@carljm
Copy link
Contributor

carljm commented Sep 10, 2012

Hmm, interesting, I run Linux locally and it passes for me here. I did wonder about whether it was a file format difference. Please do make a new pull request, that will trigger a travis run so we can make sure this fixes it before merging. Thanks!

@andreiko
Copy link
Contributor Author

It's there.

Funny that travis build 257 from this page http://travis-ci.org/#!/pypa/pip/pull_requests with the name of my latest commit actually ran on my first commit, that did not contain a test. Maybe a bug in new github-travis integration?

@qwcode qwcode mentioned this pull request Jan 27, 2013
@qwcode qwcode mentioned this pull request Sep 15, 2013
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants