Differential D3359 Diff 11910 swh/storage/cassandra/cql.py

Changeset View

Standalone View

swh/storage/cassandra/cql.py

Show First 20 Lines • Show All 651 Lines • ▼ Show 20 Lines	class CqlRunner:

_origin_visit_keys = [		_origin_visit_keys = [
"origin",		"origin",
"visit",		"visit",
"type",		"type",
"date",		"date",
]		]

@_prepared_statement("SELECT * FROM origin_visit WHERE origin = ? AND visit > ?")		@_prepared_statement(
def _origin_visit_get_no_limit(		"SELECT * FROM origin_visit WHERE origin = ? AND visit > ? "
		"ORDER BY visit ASC"
		)
		def _origin_visit_get_asc_no_limit(
self, origin_url: str, last_visit: int, *, statement		self, origin_url: str, last_visit: int, *, statement
) -> ResultSet:		) -> ResultSet:
return self._execute_with_retries(statement, [origin_url, last_visit])		return self._execute_with_retries(statement, [origin_url, last_visit])

@_prepared_statement(		@_prepared_statement(
"SELECT * FROM origin_visit WHERE origin = ? AND visit > ? LIMIT ?"		"SELECT * FROM origin_visit WHERE origin = ? AND visit > ? "
		"ORDER BY visit ASC "
		"LIMIT ?"
)		)
def _origin_visit_get_limit(		def _origin_visit_get_asc_limit(
		self, origin_url: str, last_visit: int, limit: int, *, statement
		) -> ResultSet:
		return self._execute_with_retries(statement, [origin_url, last_visit, limit])

		@_prepared_statement(
		"SELECT * FROM origin_visit WHERE origin = ? AND visit > ? "
		douarddaUnsubmitted Not Done Inline Actions shouldn't this be a `visit < ?` for pagination to work? douardda: shouldn't this be a `visit < ?` for pagination to work?
		ardumontAuthorUnsubmitted Done Inline Actions Hmm it seems you are right! So it's missing a check or something since nothing complained. ardumont: Hmm it seems you are right! So it's missing a check or something since nothing complained.
		ardumontAuthorUnsubmitted Done Inline Actions Yes, it's missing a check like: # pagination support in desc order all_visits5 = list( swh_storage.origin_visit_get( origin.url, last_visit=ov3["visit"], order="desc", limit=2) ) assert all_visits5 == [ov2, ov1] or something. ardumont: Yes, it's missing a check like: ``` # pagination support in desc order…
		ardumontAuthorUnsubmitted Done Inline Actions Actually it was missing multiple scenarios around pagination in "desc" order. And yes, once adding those, that failed as you remarked. This is now fixed in every implementations and tested appropriately (the 8 combinations of {order, pagination, limit}). ardumont: Actually it was missing multiple scenarios around pagination in "desc" order. And yes, once…
		"ORDER BY visit DESC"
		)
		def _origin_visit_get_desc_no_limit(
		self, origin_url: str, last_visit: int, *, statement
		) -> ResultSet:
		return self._execute_with_retries(statement, [origin_url, last_visit])

		@_prepared_statement(
		"SELECT * FROM origin_visit WHERE origin = ? AND visit > ? "
		"ORDER BY visit DESC "
		"LIMIT ?"
		)
		def _origin_visit_get_desc_limit(
self, origin_url: str, last_visit: int, limit: int, *, statement		self, origin_url: str, last_visit: int, limit: int, *, statement
) -> ResultSet:		) -> ResultSet:
return self._execute_with_retries(statement, [origin_url, last_visit, limit])		return self._execute_with_retries(statement, [origin_url, last_visit, limit])

def origin_visit_get(		def origin_visit_get(
self, origin_url: str, last_visit: Optional[int], limit: Optional[int]		self,
		origin_url: str,
		last_visit: Optional[int],
		limit: Optional[int],
		order: str = "asc",
) -> ResultSet:		) -> ResultSet:
		order = order.lower()
		assert order in ["asc", "desc"]

if last_visit is None:		if last_visit is None:
last_visit = -1		last_visit = -1

if limit is None:		if limit is None:
return self._origin_visit_get_no_limit(origin_url, last_visit)		method_name = f"_origin_visit_get_{order}_no_limit"
		args = [origin_url, last_visit]
else:		else:
return self._origin_visit_get_limit(origin_url, last_visit, limit)		method_name = f"_origin_visit_get_{order}_limit"
		args = [origin_url, last_visit, limit]

		method = getattr(self, method_name)
		ardumontAuthorUnsubmitted Done Inline Actions It's that way because @vlorentz convinced me that using prepared statement is faster than building the query. So there instead we define the 8 prepared statements and use them accordingly. It's also more secure against cql injection i guess. [1] https://www.scylladb.com/2017/12/13/prepared-statements-scylla/ ardumont: It's that way because @vlorentz convinced me that using prepared statement is faster than…
		rows = method(*args)
		return rows

@_prepared_insert_statement("origin_visit", _origin_visit_keys)		@_prepared_insert_statement("origin_visit", _origin_visit_keys)
def origin_visit_add_one(self, visit: OriginVisit, *, statement) -> None:		def origin_visit_add_one(self, visit: OriginVisit, *, statement) -> None:
self._add_one(statement, "origin_visit", visit, self._origin_visit_keys)		self._add_one(statement, "origin_visit", visit, self._origin_visit_keys)

_origin_visit_status_keys = [		_origin_visit_status_keys = [
"origin",		"origin",
"visit",		"visit",
"date",		"date",
"status",		"status",
"snapshot",		"snapshot",
"metadata",		"metadata",
]		]

@_prepared_insert_statement("origin_visit_status", _origin_visit_status_keys)		@_prepared_insert_statement("origin_visit_status", _origin_visit_status_keys)
def origin_visit_status_add_one(		def origin_visit_status_add_one(
self, visit_update: OriginVisitStatus, *, statement		self, visit_update: OriginVisitStatus, *, statement
) -> None:		) -> None:
assert self._origin_visit_status_keys[-1] == "metadata"		assert self._origin_visit_status_keys[-1] == "metadata"
keys = self._origin_visit_status_keys		keys = self._origin_visit_status_keys

		vlorentzUnsubmitted Not Done Inline Actions black folded the newlines but not the quotes in the prepared statement queries vlorentz: black folded the newlines but not the quotes in the prepared statement queries
		ardumontAuthorUnsubmitted Done Inline Actions ah yeah, thanks. I saw and forgot to fix those (also the flakiness in tests annoys me a bit...). ardumont: ah yeah, thanks. I saw and forgot to fix those (also the flakiness in tests annoys me a bit...).
metadata = json.dumps(visit_update.metadata)		metadata = json.dumps(visit_update.metadata)
self._execute_with_retries(		self._execute_with_retries(
statement, [getattr(visit_update, key) for key in keys[:-1]] + [metadata]		statement, [getattr(visit_update, key) for key in keys[:-1]] + [metadata]
)		)

def origin_visit_status_get_latest(self, origin: str, visit: int,) -> Optional[Row]:		def origin_visit_status_get_latest(self, origin: str, visit: int,) -> Optional[Row]:
"""Given an origin visit id, return its latest origin_visit_status		"""Given an origin visit id, return its latest origin_visit_status

▲ Show 20 Lines • Show All 189 Lines • Show Last 20 Lines