PR#87: Remove the lock when we read or move the sqlite files. - mdapi

mdapi

#87 Remove the lock when we read or move the sqlite files.

Merged 4 years ago by pingou. Opened 4 years ago by cverna.

cverna/mdapi remove_the_lock into master

Remove the lock when we read or move the sqlite files.

Clement Verna • 4 years ago

5c9950e

Dockerfile

file modified

+1 -1

		`@@ -6,7 +6,7 @@`

		`EXPOSE 8080`

		`- RUN dnf -y install python3-aiohttp python3-werkzeug python3-requests python3-sqlalchemy python3-fedora-messaging python3-flufl-lock`
		`+ RUN dnf -y install python3-aiohttp python3-werkzeug python3-requests python3-sqlalchemy python3-fedora-messaging`

		`USER 1001`
		`ENV MDAPI_CONFIG=/etc/mdapi/mdapi.cfg`

mdapi-get_repo_md

file modified

+1 -3

		`@@ -52,7 +52,6 @@`
		`from sqlalchemy import text`
		`from fedora_messaging.api import Message, publish`
		`from fedora_messaging.exceptions import PublishReturned, ConnectionException`
		`- from flufl.lock import Lock`

		`import mdapi.lib as mdapilib`

		`@@ -329,8 +328,7 @@`

		`def install_db(name, src, dest):`
		`print('%s Installing %s to %s.' % (name.ljust(padding), src, dest))`
		`- with Lock(dest + '.lock'):`
		`- shutil.move(src, dest)`
		`+ shutil.move(src, dest)`


		`def needs_update(local_file, remote_sha, sha_type):`

mdapi/__init__.py

file modified

+52 -57

		`@@ -34,7 +34,6 @@`
		`import werkzeug`
		`from aiohttp import web`
		`from multidict import MultiDict`
		`- from flufl.lock import Lock`

		`import mdapi.lib as mdapilib`

		`@@ -115,18 +114,17 @@`

		`wrongdb = False`

		`- with Lock(dbfile + '.lock'):`
		`- session = yield from mdapilib.create_session(`
		`- 'sqlite:///%s' % dbfile)`
		`- if name:`
		`- if action:`
		`- pkg = yield from mdapilib.get_package_by(`
		`- session, action, name)`
		`- else:`
		`- pkg = yield from mdapilib.get_package(session, name)`
		`- elif srcname:`
		`- pkg = yield from mdapilib.get_package_by_src(session, srcname)`
		`- session.close()`
		`+ session = yield from mdapilib.create_session(`
		`+ 'sqlite:///%s' % dbfile)`
		`+ if name:`
		`+ if action:`
		`+ pkg = yield from mdapilib.get_package_by(`
		`+ session, action, name)`
		`+ else:`
		`+ pkg = yield from mdapilib.get_package(session, name)`
		`+ elif srcname:`
		`+ pkg = yield from mdapilib.get_package_by_src(session, srcname)`
		`+ session.close()`
		`if pkg:`
		`break`

		`@@ -167,41 +165,40 @@`
		`dbfile = '%s/mdapi-%s%s-primary.sqlite' % (`
		`CONFIG['DB_FOLDER'], branch, '-%s' % repotype if repotype else '')`

		`- with Lock(dbfile + '.lock'):`
		`- session = yield from mdapilib.create_session(`
		`- 'sqlite:///%s' % dbfile)`
		`- # Fill in some extra info`
		`-`
		`- # Basic infos, always present regardless of the version of the repo`
		`- for datatype in ['conflicts', 'obsoletes', 'provides', 'requires']:`
		`- data = yield from mdapilib.get_package_info(`
		`- session, pkg.pkgKey, datatype.capitalize())`
		`- if data:`
		`- out[datatype] = [item.to_json() for item in data]`
		`- else:`
		`- out[datatype] = data`
		`-`
		`- # New meta-data present for soft dependency management in RPM`
		`- for datatype in [`
		`- 'enhances', 'recommends', 'suggests', 'supplements']:`
		`- data = yield from mdapilib.get_package_info(`
		`- session, pkg.pkgKey, datatype.capitalize())`
		`- if data:`
		`- out[datatype] = [item.to_json() for item in data]`
		`- else:`
		`- out[datatype] = data`
		`-`
		`- # Add the list of packages built from the same src.rpm`
		`- if pkg.rpm_sourcerpm:`
		`- copkgs = yield from mdapilib.get_co_packages(`
		`- session, pkg.rpm_sourcerpm)`
		`- out['co-packages'] = list(set([`
		`- cpkg.name for cpkg in copkgs`
		`- ]))`
		`+ session = yield from mdapilib.create_session(`
		`+ 'sqlite:///%s' % dbfile)`
		`+ # Fill in some extra info`
		`+`
		`+ # Basic infos, always present regardless of the version of the repo`
		`+ for datatype in ['conflicts', 'obsoletes', 'provides', 'requires']:`
		`+ data = yield from mdapilib.get_package_info(`
		`+ session, pkg.pkgKey, datatype.capitalize())`
		`+ if data:`
		`+ out[datatype] = [item.to_json() for item in data]`
		`+ else:`
		`+ out[datatype] = data`
		`+`
		`+ # New meta-data present for soft dependency management in RPM`
		`+ for datatype in [`
		`+ 'enhances', 'recommends', 'suggests', 'supplements']:`
		`+ data = yield from mdapilib.get_package_info(`
		`+ session, pkg.pkgKey, datatype.capitalize())`
		`+ if data:`
		`+ out[datatype] = [item.to_json() for item in data]`
		`else:`
		`- out['co-packages'] = []`
		`- out['repo'] = repotype if repotype else 'release'`
		`- session.close()`
		`+ out[datatype] = data`
		`+`
		`+ # Add the list of packages built from the same src.rpm`
		`+ if pkg.rpm_sourcerpm:`
		`+ copkgs = yield from mdapilib.get_co_packages(`
		`+ session, pkg.rpm_sourcerpm)`
		`+ out['co-packages'] = list(set([`
		`+ cpkg.name for cpkg in copkgs`
		`+ ]))`
		`+ else:`
		`+ out['co-packages'] = []`
		`+ out['repo'] = repotype if repotype else 'release'`
		`+ session.close()`
		`output.append(out)`
		`if singleton:`
		`return output[0]`
		`@@ -264,11 +261,10 @@`
		`if not os.path.exists(dbfile):`
		`raise web.HTTPBadRequest()`

		`- with Lock(dbfile + '.lock'):`
		`- session2 = yield from mdapilib.create_session(`
		`- 'sqlite:///%s' % dbfile)`
		`- filelist = yield from mdapilib.get_files(session2, pkg.pkgId)`
		`- session2.close()`
		`+ session2 = yield from mdapilib.create_session(`
		`+ 'sqlite:///%s' % dbfile)`
		`+ filelist = yield from mdapilib.get_files(session2, pkg.pkgId)`
		`+ session2.close()`

		`output = {`
		`'files': [fileinfo.to_json() for fileinfo in filelist],`
		`@@ -297,11 +293,10 @@`
		`if not os.path.exists(dbfile):`
		`raise web.HTTPBadRequest()`

		`- with Lock(dbfile + '.lock'):`
		`- session2 = yield from mdapilib.create_session(`
		`- 'sqlite:///%s' % dbfile)`
		`- changelogs = yield from mdapilib.get_changelog(session2, pkg.pkgId)`
		`- session2.close()`
		`+ session2 = yield from mdapilib.create_session(`
		`+ 'sqlite:///%s' % dbfile)`
		`+ changelogs = yield from mdapilib.get_changelog(session2, pkg.pkgId)`
		`+ session2.close()`

		`output = {`
		`'changelogs': [changelog.to_json() for changelog in changelogs],`

cverna commented 4 years ago

Since the cronjob never writes on the sqlite files that
are used by the web server we don't need to protect these.
The cronjob moving the file should be safe and at worst
could result in a request failing during the move.

Signed-off-by: Clement Verna cverna@tutanota.com