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

Replace cherrypy.lib.lockfile with zc.lockfile. #1729

Merged
merged 3 commits into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@jaraco
Member

jaraco commented Aug 15, 2018

I hope this change addresses #1193. Presumably supersedes #1705.

@jaraco jaraco force-pushed the bugfix/1193-zc.lockfile branch from f46c628 to 3e4917f Aug 15, 2018

@codacy-bot

This comment has been minimized.

codacy-bot commented Aug 15, 2018

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 1
- Added 3
           

See the complete overview on Codacy

from cherrypy.lib import locking
from cherrypy.lib import is_iterator
if six.PY2:
FileNotFoundError = OSError

This comment has been minimized.

@@ -553,8 +560,8 @@ def acquire_lock(self, path=None):
checker = locking.LockChecker(self.id, self.lock_timeout)
while not checker.expired():
try:
self.lock = lockfile.LockFile(path)
except lockfile.LockError:
self.lock = zc.lockfile.LockFile(path)

This comment has been minimized.

self.lock.remove()
self.lock.close()
with contextlib2.suppress(FileNotFoundError):
os.remove(self.lock._path)

This comment has been minimized.

@@ -564,8 +571,9 @@ def acquire_lock(self, path=None):
def release_lock(self, path=None):

This comment has been minimized.

@webknjaz

webknjaz Aug 15, 2018

Member

Having a path arg here looks weird to me: it's not used anywhere.

CHANGES.rst Outdated
v17.3.0
-------
* :issue:`1193`: Rely on zc.lockfile for session concurrency

This comment has been minimized.

@webknjaz

webknjaz Aug 15, 2018

Member
via :pr:`1729`

it helps a lot when I'm trying to verify things

This comment has been minimized.

@jaraco

jaraco Aug 16, 2018

Member

I'll do it this time, but I'm really disinclined to do this extra work in general and here's why: Usually, the user wishes to know the "why" of a change (the issue) and is less interested in the "how", so the issue number is usually more relevant. Moreover, the PR number is often not known at the time the PR is submitted, so requires a two phase process - commit changes, create PR, commit more changes. I want to avoid adding extra steps. As long as the changelog gives a link to a relevant issue (preferably the most relevant issue), it's a small matter from there to see the other activity that's linked, including PRs. I don't want to recommend or ascent to manual processes that replicate information that's readily available through automatic processes. I will sometimes link to a PR when there's no issue present or if that PR is somehow not readily reachable from the issue, but my preference is to link to one issue and let an investigator trace that to the PR with one extra click.

via
@codecov

This comment has been minimized.

codecov bot commented Aug 16, 2018

Codecov Report

Merging #1729 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1729      +/-   ##
=========================================
- Coverage   80.89%   80.8%   -0.09%     
=========================================
  Files         105     104       -1     
  Lines       13597   13525      -72     
=========================================
- Hits        10999   10929      -70     
+ Misses       2598    2596       -2

@jaraco jaraco merged commit 550a17a into master Aug 16, 2018

2 of 8 checks passed

LGTM analysis: Python Analysis Failed (could not build the merge commit and the base commit (73042a2))
Details
ci/circleci: linux-build CircleCI is running your tests
Details
ci/circleci: macos-build CircleCI is running your tests
Details
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
WIP ready for review
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@jaraco jaraco deleted the bugfix/1193-zc.lockfile branch Aug 16, 2018

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