D1321.id4288.diff
No OneTemporary
Actions

Size

37 KB

Subscribers

None

D1321.id4288.diff
View Options

	diff --git a/swh/storage/api/client.py b/swh/storage/api/client.py
	--- a/swh/storage/api/client.py
	+++ b/swh/storage/api/client.py
	@@ -211,18 +211,6 @@
	def fetch_history_get(self, fetch_history_id):
	return self.get('fetch_history', {'id': fetch_history_id})

	- def entity_add(self, entities):
	- return self.post('entity/add', {'entities': entities})
	-
	- def entity_get(self, uuid):
	- return self.post('entity/get', {'uuid': uuid})
	-
	- def entity_get_one(self, uuid):
	- return self.get('entity', {'uuid': uuid})
	-
	- def entity_get_from_lister_metadata(self, entities):
	- return self.post('entity/from_lister_metadata', {'entities': entities})
	-
	def stat_counters(self):
	return self.get('stat/counters')

	diff --git a/swh/storage/api/server.py b/swh/storage/api/server.py
	--- a/swh/storage/api/server.py
	+++ b/swh/storage/api/server.py
	@@ -21,25 +21,91 @@
	storage = None


	+OPERATIONS_METRIC = 'swh_storage_operations_total'
	+OPERATIONS_UNIT_METRIC = "swh_storage_operations_{unit}_total"
	+DURATION_METRIC = "swh_storage_request_duration_seconds"
	+
	+
	def timed(f):
	"""Time that function!

	"""
	@wraps(f)
	def d(a, *kw):
	- with statsd.timed('swh_storage_request_duration_seconds',
	- tags={'endpoint': f.__name__}):
	+ with statsd.timed(DURATION_METRIC, tags={'endpoint': f.__name__}):
	return f(a, *kw)

	return d


	+def encode(f):
	+ @wraps(f)
	+ def d(a, *kw):
	+ r = f(a, *kw)
	+ return encode_data(r)
	+
	+ return d
	+
	+
	+def send_metric(metric, count, method_name):
	+ """Send statsd metric with count for method `method_name`
	+
	+ If count is 0, the metric is discarded. If the metric is not
	+ parseable, the metric is discarded with a log message.
	+
	+ Args:
	+ metric (str): Metric's name (e.g content:add, content:add:bytes)
	+ count (int): Associated value for the metric
	+ method_name (str): Method's name
	+
	+ Returns:
	+ Bool to explicit if metric has been set or not
	+ """
	+ if count == 0:
	+ return False
	+
	+ metric_type = metric.split(':')
	+ _length = len(metric_type)
	+ if _length == 2:
	+ object_type, operation = metric_type
	+ metric_name = OPERATIONS_METRIC
	+ elif _length == 3:
	+ object_type, operation, unit = metric_type
	+ metric_name = OPERATIONS_UNIT_METRIC.format(unit=unit)
	+ else:
	+ logging.warning('Skipping unknown metric {%s: %s}' % (
	+ metric, count))
	+ return False
	+
	+ statsd.increment(
	+ metric_name, count, tags={
	+ 'endpoint': method_name,
	+ 'object_type': object_type,
	+ 'operation': operation,
	+ })
	+ return True
	+
	+
	+def process_metrics(f):
	+ """Increment object counters for the decorated function.
	+
	+ """
	+ @wraps(f)
	+ def d(a, *kw):
	+ r = f(a, *kw)
	+ for metric, count in r.items():
	+ send_metric(metric=metric, count=count, method_name=f.__name__)
	+
	+ return r
	+
	+ return d
	+
	+
	@app.errorhandler(Exception)
	def my_error_handler(exception):
	return error_handler(exception, encode_data)


	-@timed
	def get_storage():
	global storage
	if not storage:
	@@ -91,8 +157,10 @@

	@app.route('/content/add', methods=['POST'])
	@timed
	+@encode
	+@process_metrics
	def content_add():
	- return encode_data(get_storage().content_add(**decode_request(request)))
	+ return get_storage().content_add(**decode_request(request))


	@app.route('/content/update', methods=['POST'])
	@@ -130,8 +198,10 @@

	@app.route('/directory/add', methods=['POST'])
	@timed
	+@encode
	+@process_metrics
	def directory_add():
	- return encode_data(get_storage().directory_add(**decode_request(request)))
	+ return get_storage().directory_add(**decode_request(request))


	@app.route('/directory/path', methods=['POST'])
	@@ -150,8 +220,10 @@

	@app.route('/revision/add', methods=['POST'])
	@timed
	+@encode
	+@process_metrics
	def revision_add():
	- return encode_data(get_storage().revision_add(**decode_request(request)))
	+ return get_storage().revision_add(**decode_request(request))


	@app.route('/revision', methods=['POST'])
	@@ -182,8 +254,10 @@

	@app.route('/release/add', methods=['POST'])
	@timed
	+@encode
	+@process_metrics
	def release_add():
	- return encode_data(get_storage().release_add(**decode_request(request)))
	+ return get_storage().release_add(**decode_request(request))


	@app.route('/release', methods=['POST'])
	@@ -208,8 +282,10 @@

	@app.route('/snapshot/add', methods=['POST'])
	@timed
	+@encode
	+@process_metrics
	def snapshot_add():
	- return encode_data(get_storage().snapshot_add(**decode_request(request)))
	+ return get_storage().snapshot_add(**decode_request(request))


	@app.route('/snapshot', methods=['POST'])
	@@ -273,14 +349,20 @@

	@app.route('/origin/add_multi', methods=['POST'])
	@timed
	+@encode
	def origin_add():
	- return encode_data(get_storage().origin_add(**decode_request(request)))
	+ origins = get_storage().origin_add(**decode_request(request))
	+ send_metric('origin:add', count=len(origins), method_name='origin_add')
	+ return origins


	@app.route('/origin/add', methods=['POST'])
	@timed
	+@encode
	def origin_add_one():
	- return encode_data(get_storage().origin_add_one(**decode_request(request)))
	+ origin = get_storage().origin_add_one(**decode_request(request))
	+ send_metric('origin:add', count=1, method_name='origin_add_one')
	+ return origin


	@app.route('/origin/visit/get', methods=['POST'])
	@@ -299,9 +381,12 @@

	@app.route('/origin/visit/add', methods=['POST'])
	@timed
	+@encode
	def origin_visit_add():
	- return encode_data(get_storage().origin_visit_add(
	- **decode_request(request)))
	+ origin_visit = get_storage().origin_visit_add(
	+ **decode_request(request))
	+ send_metric('origin_visit:add', count=1, method_name='origin_visit')
	+ return origin_visit


	@app.route('/origin/visit/update', methods=['POST'])
	@@ -337,33 +422,6 @@
	get_storage().fetch_history_end(**decode_request(request)))


	-@app.route('/entity/add', methods=['POST'])
	-@timed
	-def entity_add():
	- return encode_data(
	- get_storage().entity_add(**decode_request(request)))
	-
	-
	-@app.route('/entity/get', methods=['POST'])
	-@timed
	-def entity_get():
	- return encode_data(
	- get_storage().entity_get(**decode_request(request)))
	-
	-
	-@app.route('/entity', methods=['GET'])
	-@timed
	-def entity_get_one():
	- return encode_data(get_storage().entity_get_one(request.args['uuid']))
	-
	-
	-@app.route('/entity/from_lister_metadata', methods=['POST'])
	-@timed
	-def entity_from_lister_metadata():
	- return encode_data(get_storage().entity_get_from_lister_metadata(
	- **decode_request(request)))
	-
	-
	@app.route('/tool/data', methods=['POST'])
	@timed
	def tool_get():
	@@ -373,16 +431,22 @@

	@app.route('/tool/add', methods=['POST'])
	@timed
	+@encode
	def tool_add():
	- return encode_data(get_storage().tool_add(
	- **decode_request(request)))
	+ tools = get_storage().tool_add(**decode_request(request))
	+ send_metric('tool:add', count=len(tools), method_name='tool_add')
	+ return tools


	@app.route('/origin/metadata/add', methods=['POST'])
	@timed
	+@encode
	def origin_metadata_add():
	- return encode_data(get_storage().origin_metadata_add(**decode_request(
	- request)))
	+ origin_metadata = get_storage().origin_metadata_add(
	+ **decode_request(request))
	+ send_metric(
	+ 'origin_metadata:add', count=1, method_name='origin_metadata_add')
	+ return origin_metadata


	@app.route('/origin/metadata/get', methods=['POST'])
	@@ -394,9 +458,13 @@

	@app.route('/provider/add', methods=['POST'])
	@timed
	+@encode
	def metadata_provider_add():
	- return encode_data(get_storage().metadata_provider_add(**decode_request(
	- request)))
	+ metadata_provider = get_storage().metadata_provider_add(**decode_request(
	+ request))
	+ send_metric(
	+ 'metadata_provider:add', count=1, method_name='metadata_provider')
	+ return metadata_provider


	@app.route('/provider/get', methods=['POST'])
	diff --git a/swh/storage/in_memory.py b/swh/storage/in_memory.py
	--- a/swh/storage/in_memory.py
	+++ b/swh/storage/in_memory.py
	@@ -1,4 +1,4 @@
	-# Copyright (C) 2015-2018 The Software Heritage developers
	+# Copyright (C) 2015-2019 The Software Heritage developers
	# See the AUTHORS file at the top-level directory of this distribution
	# License: GNU General Public License version 3, or any later version
	# See top-level LICENSE file for more information
	@@ -77,6 +77,16 @@
	- origin (int): if status = absent, the origin we saw the
	content in

	+ Raises:
	+ HashCollision in case of collision
	+
	+ Returns:
	+ Summary dict with the following key and associated values:
	+
	+ content:add: New contents added
	+ content_bytes:add: Sum of the contents' length data
	+ skipped_content:add: New skipped contents (no data) added
	+
	"""
	if self.journal_writer:
	for content in contents:
	@@ -84,6 +94,11 @@
	content = content.copy()
	del content['data']
	self.journal_writer.write_addition('content', content)
	+
	+ count_contents = 0
	+ count_content_added = 0
	+ count_content_bytes_added = 0
	+
	for content in contents:
	key = self._content_key(content)
	if key in self._contents:
	@@ -99,10 +114,19 @@
	self._contents[key] = copy.deepcopy(content)
	self._contents[key]['ctime'] = now()
	bisect.insort(self._sorted_sha1s, content['sha1'])
	+ count_contents += 1
	if self._contents[key]['status'] == 'visible':
	+ count_content_added += 1
	content_data = self._contents[key].pop('data')
	+ count_content_bytes_added += len(content_data)
	self.objstorage.add(content_data, content['sha1'])

	+ return {
	+ 'content:add': count_content_added,
	+ 'content:bytes:add': count_content_bytes_added,
	+ 'skipped_content:add': count_contents - count_content_added,
	+ }
	+
	def content_get(self, ids):
	"""Retrieve in bulk contents and their data.

	@@ -294,16 +318,25 @@
	- target (sha1_git): id of the object pointed at by the
	directory entry
	- perms (int): entry permissions
	+ Returns:
	+ Summary dict of keys with associated count as values:
	+
	+ directory:add: Number of directories actually added
	+
	"""
	if self.journal_writer:
	self.journal_writer.write_additions('directory', directories)

	+ count = 0
	for directory in directories:
	if directory['id'] not in self._directories:
	+ count += 1
	self._directories[directory['id']] = copy.deepcopy(directory)
	self._objects[directory['id']].append(
	('directory', directory['id']))

	+ return {'directory:add': count}
	+
	def directory_missing(self, directory_ids):
	"""List directories missing from storage

	@@ -427,10 +460,17 @@
	this revision

	date dictionaries have the form defined in :mod:`swh.model`.
	+
	+ Returns:
	+ Summary dict of keys with associated count as values
	+
	+ revision_added: New objects actually stored in db
	+
	"""
	if self.journal_writer:
	self.journal_writer.write_additions('revision', revisions)

	+ count = 0
	for revision in revisions:
	if revision['id'] not in self._revisions:
	self._revisions[revision['id']] = rev = copy.deepcopy(revision)
	@@ -441,6 +481,9 @@
	rev.get('committer_date'))
	self._objects[revision['id']].append(
	('revision', revision['id']))
	+ count += 1
	+
	+ return {'revision:add': count}

	def revision_missing(self, revision_ids):
	"""List revisions missing from storage
	@@ -518,17 +561,28 @@
	keys: name, fullname, email

	the date dictionary has the form defined in :mod:`swh.model`.
	+
	+ Returns:
	+ Summary dict of keys with associated count as values
	+
	+ release:add: New objects contents actually stored in db
	+
	"""
	if self.journal_writer:
	self.journal_writer.write_additions('release', releases)

	+ count = 0
	for rel in releases:
	- rel = copy.deepcopy(rel)
	- rel['date'] = normalize_timestamp(rel['date'])
	- self._person_add(rel['author'])
	- self._objects[rel['id']].append(
	- ('release', rel['id']))
	- self._releases[rel['id']] = rel
	+ if rel['id'] not in self._releases:
	+ rel = copy.deepcopy(rel)
	+ rel['date'] = normalize_timestamp(rel['date'])
	+ self._person_add(rel['author'])
	+ self._objects[rel['id']].append(
	+ ('release', rel['id']))
	+ self._releases[rel['id']] = rel
	+ count += 1
	+
	+ return {'release:add': count}

	def release_missing(self, releases):
	"""List releases missing from storage
	@@ -578,6 +632,12 @@

	Raises:
	ValueError: if the origin's or visit's identifier does not exist.
	+
	+ Returns:
	+ Summary dict of keys with associated count as values
	+
	+ snapshot_added: Count of object actually stored in db
	+
	"""
	if legacy_arg1:
	assert legacy_arg2
	@@ -586,6 +646,7 @@
	else:
	origin = visit = None

	+ count = 0
	for snapshot in snapshots:
	snapshot_id = snapshot['id']
	if snapshot_id not in self._snapshots:
	@@ -598,12 +659,15 @@
	'_sorted_branch_names': sorted(snapshot['branches'])
	}
	self._objects[snapshot_id].append(('snapshot', snapshot_id))
	+ count += 1

	if origin:
	# Legacy API, there can be only one snapshot
	self.origin_visit_update(
	origin, visit, snapshot=snapshots[0]['id'])

	+ return {'snapshot:add': count}
	+
	def snapshot_get(self, snapshot_id):
	"""Get the content, possibly partial, of a snapshot with the given id

	@@ -1267,7 +1331,7 @@
	- configuration (:class:`dict`): configuration of the tool,
	must be json-encodable

	- Yields:
	+ Returns:
	:class:`dict`: All the tools inserted in storage
	(including the internal ``id``). The order of the list is not
	guaranteed to match the order of the initial list.
	@@ -1283,7 +1347,7 @@
	self._tools[key] = record
	inserted.append(copy.deepcopy(self._tools[key]))

	- yield from inserted
	+ return inserted

	def tool_get(self, tool):
	"""Retrieve tool information.
	diff --git a/swh/storage/storage.py b/swh/storage/storage.py
	--- a/swh/storage/storage.py
	+++ b/swh/storage/storage.py
	@@ -114,7 +114,29 @@
	- origin (int): if status = absent, the origin we saw the
	content in

	+ Raises:
	+
	+ In case of errors, nothing is stored in the db (in the
	+ objstorage, it could though). The following exceptions can
	+ occur:
	+
	+ - HashCollision in case of collision
	+ - Any other exceptions raise by the db
	+
	+ Returns:
	+ Summary dict with the following key and associated values:
	+
	+ content:add: New contents added
	+ content:bytes:add: Sum of the contents' length data
	+ skipped_content:add: New skipped contents (no data) added
	+
	"""
	+ summary = {
	+ 'content:add': 0,
	+ 'content:bytes:add': 0,
	+ 'skipped_content:add': 0,
	+ }
	+
	if self.journal_writer:
	for item in content:
	if 'data' in item:
	@@ -137,7 +159,8 @@
	for d in content:
	if 'status' not in d:
	d['status'] = 'visible'
	- if 'length' not in d:
	+ length = d.get('length')
	+ if length is None:
	d['length'] = -1
	content_by_status[d['status']].append(d)

	@@ -150,12 +173,29 @@
	content_without_data))

	def add_to_objstorage():
	- data = {
	- cont['sha1']: cont['data']
	- for cont in content_with_data
	- if cont['sha1'] in missing_content
	- }
	+ """Add to objstorage the new missing_content
	+
	+ Returns:
	+ Sum of all the content's data length pushed to the
	+ objstorage. No filtering is done on contents here, so
	+ we might send over multiple times the same content and
	+ count as many times the contents' raw length bytes.
	+
	+ """
	+ content_bytes_added = 0
	+ data = {}
	+ for cont in content_with_data:
	+ sha1 = cont['sha1']
	+ seen = data.get(sha1)
	+ if sha1 in missing_content and not seen:
	+ data[sha1] = cont['data']
	+ content_bytes_added += cont['length']
	+
	+ # FIXME: Since we do the filtering anyway now, we might as
	+ # well make the objstorage's add_batch call return what we
	+ # want here (real bytes added)... that'd simplify this...
	self.objstorage.add_batch(data)
	+ return content_bytes_added

	with db.transaction() as cur:
	with ThreadPoolExecutor(max_workers=1) as executor:
	@@ -177,17 +217,19 @@
	from . import HashCollision
	if e.diag.sqlstate == '23505' and \
	e.diag.table_name == 'content':
	- constaint_to_hash_name = {
	+ constraint_to_hash_name = {
	'content_pkey': 'sha1',
	'content_sha1_git_idx': 'sha1_git',
	'content_sha256_idx': 'sha256',
	}
	- colliding_hash_name = constaint_to_hash_name \
	+ colliding_hash_name = constraint_to_hash_name \
	.get(e.diag.constraint_name)
	raise HashCollision(colliding_hash_name)
	else:
	raise

	+ summary['content:add'] = len(missing_content)
	+
	if missing_skipped:
	missing_filtered = (
	cont for cont in content_without_data
	@@ -200,10 +242,14 @@

	# move metadata in place
	db.skipped_content_add_from_temp(cur)
	+ summary['skipped_content:add'] = len(missing_skipped)

	# Wait for objstorage addition before returning from the
	# transaction, bubbling up any exception
	- added_to_objstorage.result()
	+ content_bytes_added = added_to_objstorage.result()
	+
	+ summary['content:bytes:add'] = content_bytes_added
	+ return summary

	@db_transaction()
	def content_update(self, content, keys=[], db=None, cur=None):
	@@ -449,7 +495,14 @@
	- target (sha1_git): id of the object pointed at by the
	directory entry
	- perms (int): entry permissions
	+
	+ Returns:
	+ Summary dict of keys with associated count as values:
	+
	+ directory:add: Number of directories actually added
	+
	"""
	+ summary = {'directory:add': 0}
	if self.journal_writer:
	self.journal_writer.write_additions('directory', directories)

	@@ -470,7 +523,7 @@

	dirs_missing = set(self.directory_missing(dirs))
	if not dirs_missing:
	- return
	+ return summary

	db = self.get_db()
	with db.transaction() as cur:
	@@ -498,6 +551,9 @@

	# Do the final copy
	db.directory_add_from_temp(cur)
	+ summary['directory:add'] = len(dirs_missing)
	+
	+ return summary

	@db_transaction_generator()
	def directory_missing(self, directories, db=None, cur=None):
	@@ -583,7 +639,15 @@
	this revision

	date dictionaries have the form defined in :mod:`swh.model`.
	+
	+ Returns:
	+ Summary dict of keys with associated count as values
	+
	+ revision:add: New objects actually stored in db
	+
	"""
	+ summary = {'revision:add': 0}
	+
	if self.journal_writer:
	self.journal_writer.write_additions('revision', revisions)

	@@ -593,7 +657,7 @@
	set(revision['id'] for revision in revisions)))

	if not revisions_missing:
	- return
	+ return summary

	with db.transaction() as cur:
	db.mktemp_revision(cur)
	@@ -614,6 +678,8 @@
	db.copy_to(parents_filtered, 'revision_history',
	['id', 'parent_id', 'parent_rank'], cur)

	+ return {'revision:add': len(revisions_missing)}
	+
	@db_transaction_generator()
	def revision_missing(self, revisions, db=None, cur=None):
	"""List revisions missing from storage
	@@ -707,7 +773,15 @@
	keys: name, fullname, email

	the date dictionary has the form defined in :mod:`swh.model`.
	+
	+ Returns:
	+ Summary dict of keys with associated count as values
	+
	+ release:add: New objects contents actually stored in db
	+
	"""
	+ summary = {'release:add': 0}
	+
	if self.journal_writer:
	self.journal_writer.write_additions('release', releases)

	@@ -717,7 +791,7 @@
	releases_missing = set(self.release_missing(release_ids))

	if not releases_missing:
	- return
	+ return summary

	with db.transaction() as cur:
	db.mktemp_release(cur)
	@@ -732,6 +806,8 @@

	db.release_add_from_temp(cur)

	+ return {'release:add': len(releases_missing)}
	+
	@db_transaction_generator()
	def release_missing(self, releases, db=None, cur=None):
	"""List releases missing from storage
	@@ -793,6 +869,13 @@

	Raises:
	ValueError: if the origin or visit id does not exist.
	+
	+ Returns:
	+
	+ Summary dict of keys with associated count as values
	+
	+ snapshot:add: Count of object actually stored in db
	+
	"""
	if origin:
	if not visit:
	@@ -814,6 +897,7 @@

	created_temp_table = False

	+ count = 0
	for snapshot in snapshots:
	if not db.snapshot_exists(snapshot['id'], cur):
	if not created_temp_table:
	@@ -839,6 +923,7 @@
	self.journal_writer.write_addition('snapshot', snapshot)

	db.snapshot_add(snapshot['id'], cur)
	+ count += 1

	if visit_id:
	# Legacy API, there can be only one snapshot
	@@ -846,6 +931,8 @@
	origin_id, visit_id, snapshot=snapshots[0]['id'],
	db=db, cur=cur)

	+ return {'snapshot:add': count}
	+
	@db_transaction(statement_timeout=2000)
	def snapshot_get(self, snapshot_id, db=None, cur=None):
	"""Get the content, possibly partial, of a snapshot with the given id
	@@ -1484,7 +1571,7 @@
	for line in db.origin_metadata_get_by(origin_id, provider_type, cur):
	yield dict(zip(db.origin_metadata_get_cols, line))

	- @db_transaction_generator()
	+ @db_transaction()
	def tool_add(self, tools, db=None, cur=None):
	"""Add new tools to the storage.

	@@ -1497,7 +1584,7 @@
	- configuration (:class:`dict`): configuration of the tool,
	must be json-encodable

	- Yields:
	+ Returns:
	:class:`dict`: All the tools inserted in storage
	(including the internal ``id``). The order of the list is not
	guaranteed to match the order of the initial list.
	@@ -1509,8 +1596,7 @@
	cur)

	tools = db.tool_add_from_temp(cur)
	- for line in tools:
	- yield dict(zip(db.tool_cols, line))
	+ return [dict(zip(db.tool_cols, line)) for line in tools]

	@db_transaction(statement_timeout=500)
	def tool_get(self, tool, db=None, cur=None):
	diff --git a/swh/storage/tests/test_server.py b/swh/storage/tests/test_server.py
	--- a/swh/storage/tests/test_server.py
	+++ b/swh/storage/tests/test_server.py
	@@ -7,7 +7,12 @@
	import pytest
	import yaml

	-from swh.storage.api.server import load_and_check_config
	+from unittest.mock import patch
	+
	+from swh.storage.api.server import (
	+ load_and_check_config, send_metric,
	+ OPERATIONS_METRIC, OPERATIONS_UNIT_METRIC
	+)


	def prepare_config_file(tmpdir, content, name='config.yml'):
	@@ -127,3 +132,44 @@
	cfg = load_and_check_config(config_path, type='any')

	assert cfg == config
	+
	+
	+def test_send_metric_unknown_unit():
	+ r = send_metric('content', count=10, method_name='content_add')
	+ assert r is False
	+ r = send_metric('sthg:add:bytes:extra', count=10, method_name='sthg_add')
	+ assert r is False
	+
	+
	+def test_send_metric_no_value():
	+ r = send_metric('content:add', count=0, method_name='content_add')
	+ assert r is False
	+
	+
	+@patch('swh.storage.api.server.statsd.increment')
	+def test_send_metric_no_unit(mock_statsd):
	+ r = send_metric('content:add', count=10, method_name='content_add')
	+
	+ mock_statsd.assert_called_with(OPERATIONS_METRIC, 10, tags={
	+ 'endpoint': 'content_add',
	+ 'object_type': 'content',
	+ 'operation': 'add',
	+ })
	+
	+ assert r
	+
	+
	+@patch('swh.storage.api.server.statsd.increment')
	+def test_send_metric_unit(mock_statsd):
	+ unit_ = 'bytes'
	+ r = send_metric('c:add:%s' % unit_, count=100, method_name='c_add')
	+
	+ expected_metric = OPERATIONS_UNIT_METRIC.format(unit=unit_)
	+ mock_statsd.assert_called_with(
	+ expected_metric, 100, tags={
	+ 'endpoint': 'c_add',
	+ 'object_type': 'c',
	+ 'operation': 'add',
	+ })
	+
	+ assert r
	diff --git a/swh/storage/tests/test_storage.py b/swh/storage/tests/test_storage.py
	--- a/swh/storage/tests/test_storage.py
	+++ b/swh/storage/tests/test_storage.py
	@@ -1,4 +1,4 @@
	-# Copyright (C) 2015-2018 The Software Heritage developers
	+# Copyright (C) 2015-2019 The Software Heritage developers
	# See the AUTHORS file at the top-level directory of this distribution
	# License: GNU General Public License version 3, or any later version
	# See top-level LICENSE file for more information
	@@ -552,7 +552,13 @@
	def test_content_add(self):
	cont = self.cont

	- self.storage.content_add([cont])
	+ actual_result = self.storage.content_add([cont])
	+ self.assertEqual(actual_result, {
	+ 'content:add': 1,
	+ 'content:bytes:add': cont['length'],
	+ 'skipped_content:add': 0
	+ })
	+
	self.assertEqual(list(self.storage.content_get([cont['sha1']])),
	[{'sha1': cont['sha1'], 'data': cont['data']}])

	@@ -561,10 +567,38 @@
	self.assertEqual(list(self.journal_writer.objects),
	[('content', expected_cont)])

	+ def test_content_add_same_input(self):
	+ cont = self.cont
	+
	+ actual_result = self.storage.content_add([cont, cont])
	+ self.assertEqual(actual_result, {
	+ 'content:add': 1,
	+ 'content:bytes:add': cont['length'],
	+ 'skipped_content:add': 0
	+ })
	+
	+ def test_content_add_different_input(self):
	+ cont = self.cont
	+ cont2 = self.cont2
	+
	+ actual_result = self.storage.content_add([cont, cont2])
	+ self.assertEqual(actual_result, {
	+ 'content:add': 2,
	+ 'content:bytes:add': cont['length'] + cont2['length'],
	+ 'skipped_content:add': 0
	+ })
	+
	def test_content_add_db(self):
	cont = self.cont

	- self.storage.content_add([cont])
	+ actual_result = self.storage.content_add([cont])
	+
	+ self.assertEqual(actual_result, {
	+ 'content:add': 1,
	+ 'content:bytes:add': cont['length'],
	+ 'skipped_content:add': 0
	+ })
	+
	if hasattr(self.storage, 'objstorage'):
	self.assertIn(cont['sha1'], self.storage.objstorage)
	self.cursor.execute('SELECT sha1, sha1_git, sha256, length, status'
	@@ -601,7 +635,13 @@
	cont2 = self.skipped_cont2.copy()
	cont2['blake2s256'] = None

	- self.storage.content_add([cont, cont, cont2])
	+ actual_result = self.storage.content_add([cont, cont, cont2])
	+
	+ self.assertEqual(actual_result, {
	+ 'content:add': 0,
	+ 'content:bytes:add': 0,
	+ 'skipped_content:add': 2,
	+ })

	self.cursor.execute('SELECT sha1, sha1_git, sha256, blake2s256, '
	'length, status, reason '
	@@ -722,7 +762,9 @@
	init_missing = list(self.storage.directory_missing([self.dir['id']]))
	self.assertEqual([self.dir['id']], init_missing)

	- self.storage.directory_add([self.dir])
	+ actual_result = self.storage.directory_add([self.dir])
	+ self.assertEqual(actual_result, {'directory:add': 1})
	+
	self.assertEqual(list(self.journal_writer.objects),
	[('directory', self.dir)])

	@@ -737,7 +779,10 @@
	init_missing = list(self.storage.directory_missing([self.dir['id']]))
	self.assertEqual([self.dir['id']], init_missing)

	- self.storage.directory_add([self.dir, self.dir2, self.dir3])
	+ actual_result = self.storage.directory_add(
	+ [self.dir, self.dir2, self.dir3])
	+ self.assertEqual(actual_result, {'directory:add': 3})
	+
	self.assertEqual(list(self.journal_writer.objects),
	[('directory', self.dir),
	('directory', self.dir2),
	@@ -765,7 +810,8 @@
	init_missing = list(self.storage.directory_missing([self.dir3['id']]))
	self.assertEqual([self.dir3['id']], init_missing)

	- self.storage.directory_add([self.dir3])
	+ actual_result = self.storage.directory_add([self.dir3])
	+ self.assertEqual(actual_result, {'directory:add': 1})

	expected_entries = [
	{
	@@ -825,7 +871,8 @@
	init_missing = self.storage.revision_missing([self.revision['id']])
	self.assertEqual([self.revision['id']], list(init_missing))

	- self.storage.revision_add([self.revision])
	+ actual_result = self.storage.revision_add([self.revision])
	+ self.assertEqual(actual_result, {'revision:add': 1})

	end_missing = self.storage.revision_missing([self.revision['id']])
	self.assertEqual([], list(end_missing))
	@@ -833,6 +880,10 @@
	self.assertEqual(list(self.journal_writer.objects),
	[('revision', self.revision)])

	+ # already there so nothing added
	+ actual_result = self.storage.revision_add([self.revision])
	+ self.assertEqual(actual_result, {'revision:add': 0})
	+
	def test_revision_log(self):
	# given
	# self.revision4 -is-child-of-> self.revision3
	@@ -945,7 +996,8 @@
	self.assertEqual([self.release['id'], self.release2['id']],
	list(init_missing))

	- self.storage.release_add([self.release, self.release2])
	+ actual_result = self.storage.release_add([self.release, self.release2])
	+ self.assertEqual(actual_result, {'release:add': 2})

	end_missing = self.storage.release_missing([self.release['id'],
	self.release2['id']])
	@@ -955,6 +1007,10 @@
	[('release', self.release),
	('release', self.release2)])

	+ # already present so nothing added
	+ actual_result = self.storage.release_add([self.release, self.release2])
	+ self.assertEqual(actual_result, {'release:add': 0})
	+
	def test_release_get(self):
	# given
	self.storage.release_add([self.release, self.release2])
	@@ -1432,7 +1488,9 @@
	self.date_visit1)
	visit_id = origin_visit1['visit']

	- self.storage.snapshot_add([self.empty_snapshot])
	+ actual_result = self.storage.snapshot_add([self.empty_snapshot])
	+ self.assertEqual(actual_result, {'snapshot:add': 1})
	+
	self.storage.origin_visit_update(
	origin_id, visit_id, snapshot=self.empty_snapshot['id'])

	@@ -1510,7 +1568,9 @@
	self.date_visit1)
	visit_id = origin_visit1['visit']

	- self.storage.snapshot_add(origin_id, visit_id, self.complete_snapshot)
	+ actual_result = self.storage.snapshot_add(
	+ origin_id, visit_id, self.complete_snapshot)
	+ self.assertEqual(actual_result, {'snapshot:add': 1})

	by_id = self.storage.snapshot_get(self.complete_snapshot['id'])
	self.assertEqual(by_id, self.complete_snapshot)
	@@ -1519,7 +1579,9 @@
	self.assertEqual(by_ov, self.complete_snapshot)

	def test_snapshot_add_many(self):
	- self.storage.snapshot_add([self.snapshot, self.complete_snapshot])
	+ actual_result = self.storage.snapshot_add(
	+ [self.snapshot, self.complete_snapshot])
	+ self.assertEqual(actual_result, {'snapshot:add': 2})

	self.assertEqual(
	self.complete_snapshot,
	@@ -1530,8 +1592,12 @@
	self.storage.snapshot_get(self.snapshot['id']))

	def test_snapshot_add_many_incremental(self):
	- self.storage.snapshot_add([self.complete_snapshot])
	- self.storage.snapshot_add([self.snapshot, self.complete_snapshot])
	+ actual_result = self.storage.snapshot_add([self.complete_snapshot])
	+ self.assertEqual(actual_result, {'snapshot:add': 1})
	+
	+ actual_result2 = self.storage.snapshot_add(
	+ [self.snapshot, self.complete_snapshot])
	+ self.assertEqual(actual_result2, {'snapshot:add': 1})

	self.assertEqual(
	self.complete_snapshot,
	@@ -1547,7 +1613,9 @@
	self.date_visit1)
	visit_id = origin_visit1['visit']

	- self.storage.snapshot_add(origin_id, visit_id, self.complete_snapshot)
	+ actual_result = self.storage.snapshot_add(
	+ origin_id, visit_id, self.complete_snapshot)
	+ self.assertEqual(actual_result, {'snapshot:add': 1})

	snp_id = self.complete_snapshot['id']
	snp_size = self.storage.snapshot_count_branches(snp_id)
	@@ -2213,7 +2281,7 @@
	self.assertIsNone(actual_tool) # does not exist

	# add it
	- actual_tools = list(self.storage.tool_add([tool]))
	+ actual_tools = self.storage.tool_add([tool])

	self.assertEqual(len(actual_tools), 1)
	actual_tool = actual_tools[0]
	@@ -2221,7 +2289,7 @@
	new_id = actual_tool.pop('id')
	self.assertEqual(actual_tool, tool)

	- actual_tools2 = list(self.storage.tool_add([tool]))
	+ actual_tools2 = self.storage.tool_add([tool])
	actual_tool2 = actual_tools2[0]
	self.assertIsNotNone(actual_tool2) # now it exists
	new_id2 = actual_tool2.pop('id')
	@@ -2245,7 +2313,7 @@
	'configuration': {},
	}]

	- actual_tools = list(self.storage.tool_add(new_tools))
	+ actual_tools = self.storage.tool_add(new_tools)
	self.assertEqual(len(actual_tools), 2)

	# order not guaranteed, so we iterate over results to check
	@@ -2283,7 +2351,7 @@
	'configuration': {"type": "local", "context": "npm"},
	}

	- tools = list(self.storage.tool_add([tool]))
	+ tools = self.storage.tool_add([tool])
	expected_tool = tools[0]

	# when
	@@ -2339,7 +2407,7 @@
	origin_metadata0 = list(self.storage.origin_metadata_get_by(origin_id))
	self.assertTrue(len(origin_metadata0) == 0)

	- tools = list(self.storage.tool_add([self.metadata_tool]))
	+ tools = self.storage.tool_add([self.metadata_tool])
	tool = tools[0]

	self.storage.metadata_provider_add(
	@@ -2377,7 +2445,7 @@
	'provider_name': self.provider['name'],
	'provider_url': self.provider['url']
	})
	- tool = list(self.storage.tool_add([self.metadata_tool]))[0]
	+ tool = self.storage.tool_add([self.metadata_tool])[0]
	# when adding for the same origin 2 metadatas
	self.storage.origin_metadata_add(
	origin_id,
	@@ -2466,7 +2534,7 @@

	# using the only tool now inserted in the data.sql, but for this
	# provider should be a crawler tool (not yet implemented)
	- tool = list(self.storage.tool_add([self.metadata_tool]))[0]
	+ tool = self.storage.tool_add([self.metadata_tool])[0]

	# when adding for the same origin 2 metadatas
	self.storage.origin_metadata_add(

File Metadata

Mime Type: text/plain
Expires: Mar 17 2025, 7:17 PM (7 w, 3 d ago)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 3215446

D1321.id4288.diffNo OneTemporaryActions

D1321.id4288.diffView Options

File Metadata

Event Timeline

D1321.id4288.diff
No OneTemporary
Actions

D1321.id4288.diff
View Options