Libertine

Merge lp:~larryprice/libertine/better-db-locking into lp:libertine

better-db-locking
Merge into devel

Proposed by Larry Price on 2017-03-29

Status:

Merged

Approved by:

Christopher Townsend on 2017-03-29

Approved revision:

415

Merged at revision:

453

Proposed branch:

lp:~larryprice/libertine/better-db-locking

Merge into:

lp:libertine

Diff against target:

119 lines (+52/-17)

1 file modified

python/libertine/ContainersConfig.py (+52/-17)

To merge this branch:

bzr merge lp:~larryprice/libertine/better-db-locking

Medium

Fix Committed

Link a bug report

Reviewer	Review Type	Date Requested	Status
Christopher Townsend		2017-03-29	Approve on 2017-03-29
Libertine CI Bot	continuous-integration		Approve on 2017-03-29
Review via email: mp+321329@code.launchpad.net

Commit message

Reopen the database file after every failure to grab the lock.

Description of the change

Reopen the database file after every failure to grab the lock.

Revision history for this message

Libertine CI Bot (libertine-ci-bot) wrote on 2017-03-29:

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-ci/493/rebuild

review: Approve (continuous-integration)

Revision history for this message

Christopher Townsend (townsend) wrote on 2017-03-29:

Nice, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Larry Price

Libertine Developers

 === modified file 'python/libertine/ContainersConfig.py'
 --- python/libertine/ContainersConfig.py	2017-02-23 21:08:28 +0000
 +++ python/libertine/ContainersConfig.py	2017-03-29 16:52:33 +0000
@@ -14,42 +14,77 @@
  import fcntl
  import json
--import libertine.utils
  import os
  import sys
++import errno
++import time
++
  from hashlib import md5
--from libertine.HostInfo import HostInfo
++from . import utils, HostInfo
++
++class _ContainersConfigFile(object):
++    def __init__(self, readonly=True):
++        self._readonly = readonly
++
++        if readonly:
++            self._flags = 'r'
++        else:
++            self._flags = 'w'
++
++        self._fd = None
++
++    def __enter__(self):
++        container_config_file = utils.get_libertine_database_file_path()
++        if self._readonly and (not os.path.exists(container_config_file) or
++                               os.path.getsize(container_config_file) == 0):
++            return None
++
++        retries = 0
++        while retries < 100:
++            try:
++                fd = open(container_config_file, self._flags)
++                fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
++
++                self._fd = fd
++                return self._fd
++            except IOError as e:
++                if e.errno != errno.EAGAIN:
++                    raise
++                else:
++                    fd.close()
++                    retries += 1
++                    time.sleep(0.05)
++
++    def __exit__(self, *args):
++        if self._fd:
++            self._fd.close()
  def read_container_config_file():
      container_list = {}
--    container_config_file = libertine.utils.get_libertine_database_file_path()
--
--    if (os.path.exists(container_config_file) and
--        os.path.getsize(container_config_file) != 0):
--        with open(container_config_file, 'r') as fd:
--            fcntl.flock(fd, fcntl.LOCK_EX)
++    with _ContainersConfigFile() as fd:
++        if fd is not None:
              container_list = json.load(fd)
      return container_list
  def write_container_config_file(container_list):
--    container_config_file = libertine.utils.get_libertine_database_file_path()
++    container_config_file = utils.get_libertine_database_file_path()
      # Add a warning to adventurous users advising against mucking with this file
      if container_list is not None:
          container_list["_warning"] = "This file is automatically generated by Libertine and should not be manually edited."
--    with open(container_config_file, 'w') as fd:
--        fcntl.flock(fd, fcntl.LOCK_EX)
--        json.dump(container_list, fd, sort_keys=True, indent=4)
--        fd.write('\n')
++    with _ContainersConfigFile(False) as fd:
++        if fd:
++            json.dump(container_list, fd, sort_keys=True, indent=4)
++            fd.write('\n')
  def container_config_hash():
      checksum = md5()
--    container_config_file = libertine.utils.get_libertine_database_file_path()
++    container_config_file = utils.get_libertine_database_file_path()
      if (os.path.exists(container_config_file) and os.path.getsize(container_config_file) != 0):
          with open(container_config_file, "rb") as fd:
              fcntl.flock(fd, fcntl.LOCK_EX)
@@ -214,12 +249,12 @@
      def check_container_id(self, container_id):
          if container_id and not self.container_exists(container_id):
--            libertine.utils.get_logger().error("Container id \'%s\' does not exist." % container_id)
++            utils.get_logger().error("Container id '{container_id}' does not exist.".format(container_id=container_id))
              sys.exit(1)
          elif not container_id:
              container_id = self.get_default_container_id()
              if container_id is None:
--                libertine.utils.get_logger().error("No default container available.")
++                utils.get_logger().error("No default container available.")
                  sys.exit(1)
          return container_id
@@ -263,7 +298,7 @@
      def delete_container(self, container_id):
          if not self.container_list:
--            libertine.utils.get_logger().error("Unable to delete container.  No containers defined.")
++            utils.get_logger().error("Unable to delete container. No containers defined.")
              sys.exit(1)
          container = self._get_container_entry(container_id)