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

Remove Lockfile #7023

Merged
merged 7 commits into from
Sep 19, 2019
Merged

Conversation

chrahunt
Copy link
Member

Following up on #6954, our HTTP cache now uses write-then-move instead of explicit file locking. This was the last place that Lockfile was used, so it has been removed.

Closes #4766.

@chrahunt chrahunt added the type: maintenance Related to Development and Maintenance Processes label Sep 15, 2019
@chrahunt
Copy link
Member Author

Test failure looks unrelated.

@chrahunt chrahunt closed this Sep 15, 2019
@chrahunt chrahunt reopened this Sep 15, 2019
@chrahunt
Copy link
Member Author

chrahunt commented Sep 16, 2019

@cjerdonek, any ideas why TestCandidateEvaluator.test_sort_best_candidate__best_yanked_but_not_all (from here) would be failing here but not in other PRs/master?

Windows Python 2.7 x86 (log) and Python 3.7 x64 (log) both fail in this test. It appears that some output from pep425tags which traces at DEBUG level is getting captured. caplog should not be changing the default level (WARNING) unless explicitly asked so I'm pretty sure it's unrelated to the test itself. Maybe PIP_VERBOSE is set somewhere?

@cjerdonek
Copy link
Member

cjerdonek commented Sep 16, 2019

@cjerdonek, any ideas why TestCandidateEvaluator.test_sort_best_candidate__best_yanked_but_not_all would be failing here but not in other PRs/master?

Yes, I noticed this on another PR, too. I believe this is due to the test ordering being different (probably due e.g. to the introduction of different test modules combined with platform-dependent skipped tests, etc) combined with the tests not being completely isolated. A possible quick fix is to set the appropriate log level on caplog at the beginning of the errant tests (probably caplog.set_level(logging.INFO)). And the longer term fix is to see if pytest has a way to automatically reset caplog's level to a constant after (or before) each test. I'm sure pytest has some capability like this, but I didn't look into it yet.

In the absence of a pytest feature like that, we would need to reset caplog at the end of each test that changes it (e.g. using the context manager form).

@pradyunsg
Copy link
Member

In the absence of a pytest feature like that, we would need to reset caplog at the end of each test that changes it (e.g. using the context manager form).

I think it'd be better to override the fixture, the same way we override tmpdir.

@cjerdonek
Copy link
Member

Hmm, the pytest docs say it already resets the caplog level at the end of every test: https://docs.pytest.org/en/latest/logging.html#caplog-fixture
And yet manually setting the level at the beginning of the affected test worked for me when I noticed this. So I'm not sure why pytest doesn't appear to be behaving as documented.

@chrahunt
Copy link
Member Author

chrahunt commented Sep 16, 2019

I put that workaround in place for the failing test and created #7026 to track the investigation and fix for the underlying cause.

@chrahunt chrahunt marked this pull request as ready for review September 16, 2019 18:46
@chrahunt chrahunt merged commit b0a6428 into pypa:master Sep 19, 2019
@chrahunt chrahunt deleted the maint/remove-lockfile-dependency branch September 19, 2019 04:24
@chrahunt
Copy link
Member Author

Thanks for reviewing @pradyunsg!

@xavfernandez xavfernandez mentioned this pull request Oct 9, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Oct 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 19, 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 type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moving off lockfile
4 participants