Summary:

Add cached statistics for LocalSites.

Review Request #12303 — Created May 26, 2022 and submitted June 8, 2022, 3:01 p.m.

Information
Owner:	chipx86
Repository:	Review Board
Branch:	release-5.0.x
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

This introduces LocalSiteManager, which is the new manager for

LocalSite objects. It provides methods used to quickly make

determinations about the Local Site usage on the server.
The main method currently is get_stats(), which returns a dictionary

containing:

The total number of Local Sites on the server
The number of public Local Sites
The number of private Local Sites
A UUID representing the generated state (which can be used as a

   component of other cache keys to enable automatic invalidation)

This is cached and invalidated any time a LocalSite is created,

deleted, or if the public state has changed, making it fast and

reliable for lookups.
There are additional wrappers around this that leverage the cached

state. These include:

has_local_sites()
has_public_local_sites()
has_private_local_sites()

These will soon be used to make some parts of our queries conditional on

their presence, letting us simplify queries in the common case.

Testing Done

Unit tests pass.

Commits

Summary
	Add cached statistics for LocalSites.

Screenshots

Issues

Description	From	Last Updated
I've noticed there's different ways you write out the class for :py:class in the doc strings: 1. the way on …	maubin	June 1, 2022, 2:50 p.m.
I think this is supposed to be name=self.local_site_name. Also instead it might be good to change the default name value …	maubin	May 30, 2022, 9:45 a.m.

flake8 passed.

JSHint passed.

Change Summary:


Added state_uuid to the get_stats() results, which can be used for other cache keys for automatic invalidation.
Removed some effectively duplicate unit tests around invalidation.

Description:

		This introduces `LocalSiteManager`, which is the new manager for
~		`LocalSite` objects. It provides a `get_stat()` method that can be used
~		to gather the counts on total, public, or private `LocalSite`s in the
	~	`LocalSite` objects. It provides methods used to quickly make
	~	determinations about the Local Site usage on the server.
-		database. This is cached and invalidated any time a `LocalSite` is
-		created, deleted, or if the `public` state has changed, making it fast
-		and reliable for lookups.

~		There are wrappers for quickly checking if there are any `LocalSite`s in
~		the database at all, or if there are any public or private ones. These
~		will soon be used to make some parts of our queries conditional on their
~		presence, letting us simplify queries in the common case.
	~	The main method currently is `get_stat()`, which returns a dictionary
	~	containing:
	~
	~	The total number of Local Sites on the server
	+	The number of public Local Sites
	+	The number of private Local Sites
	+	A UUID representing the generated state (which can be used as a component of other cache keys to enable automatic invalidation)
	+
	+	This is cached and invalidated any time a `LocalSite` is created,
	+	deleted, or if the `public` state has changed, making it fast and
	+	reliable for lookups.
	+
	+	There are additional wrappers around this that leverage the cached
	+	state. These include:
	+
	+	`has_local_sites()`
	+	`has_public_local_sites()`
	+	`has_private_local_sites()`
	+
	+	These will soon be used to make some parts of our queries conditional on
	+	their presence, letting us simplify queries in the common case.

Commits:

	Summary
-		Add cached statistics for LocalSites.
+		Add cached statistics for LocalSites.

Diff:

Revision 2 (+1068 -6)

Show changes

	reviewboard/site/managers.py
	reviewboard/site/models.py
	reviewboard/site/signal_handlers.py
	reviewboard/site/tests/test_local_site_manager.py
	reviewboard/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Description:

		This introduces `LocalSiteManager`, which is the new manager for
		`LocalSite` objects. It provides methods used to quickly make
		determinations about the Local Site usage on the server.

~		The main method currently is `get_stat()`, which returns a dictionary
	~	The main method currently is `get_stats()`, which returns a dictionary
		containing:

		The total number of Local Sites on the server
		The number of public Local Sites
		The number of private Local Sites
		A UUID representing the generated state (which can be used as a component of other cache keys to enable automatic invalidation)

		This is cached and invalidated any time a `LocalSite` is created,
		deleted, or if the `public` state has changed, making it fast and
		reliable for lookups.

		There are additional wrappers around this that leverage the cached
		state. These include:

		`has_local_sites()`
		`has_public_local_sites()`
		`has_private_local_sites()`

		These will soon be used to make some parts of our queries conditional on
		their presence, letting us simplify queries in the common case.

reviewboard/site/managers.py (Diff revision 1)

Show all issues

I've noticed there's different ways you write out the class for :py:class in the doc strings:

1. the way on this line

2. ":py:class:LocalSite" on line 59

3. ":py:class:~reviewboard.site.models.LocalSite" on line 185
What's the difference between these and why use different ways?

chipx86 May 30, 2022, 12:54 a.m.

This way's used because I want to use a different name than the class name ("LocalSites" instead of "LocalSite").
Way #2 is wrong (I'll fix that) because LocalSite is not defined in the current module. It'd be fine if this was used in models.py.
Way #3 is correct when in a different module and wanting to use the class's name. The ~ at the start prevents the name shown from having the full class path.

maubin May 30, 2022, 9:45 a.m.
```
got it, cool
```

reviewboard/testing/testcase.py (Diff revision 1)

Show all issues

I think this is supposed to be name=self.local_site_name. Also instead it might be good to change the default name value to a LocalSite that doesn't exist, so that the default use doesn't trigger the assert?

chipx86 May 30, 2022, 12:54 a.m.

In the scope of a function definition, you can't use self. It'll reference class-level attributes instead (this is defined on the class as an attribute that can be overridden).
This will only assert by default if using the test_site fixture, in which case you probably won't want to call create_local_site(). This is more useful if avoiding usage of fixtures.

maubin May 30, 2022, 9:45 a.m.
```
ah i see, makes sense
```

Change Summary:

Updated a :py:class: reference to LocalSite to use the full class path.

Commits:

	Summary
-		Add cached statistics for LocalSites.
+		Add cached statistics for LocalSites.

Diff:

Revision 3 (+1070 -6)

Show changes

	reviewboard/site/managers.py
	reviewboard/site/models.py
	reviewboard/site/signal_handlers.py
	reviewboard/site/tests/test_local_site_manager.py
	reviewboard/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (1f37555)