Summary

Switch out ConcurrencyManager for Manager.

Review Request #14073 — Created Aug. 4, 2024 and submitted Aug. 8, 2024, 9:48 a.m.

Information

Owner

david

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Early on in Django history, the built-in Manager.get_or_create method
was racy, potentially raising IntegrityError. We had built a
ConcurrencyManager to handle this case, and used it for many of our
models where we encountered issues.

As of quite some time ago, Django's built-in Manager does exactly the
same thing as our ConcurrencyManager did, and we just didn't discover
that because everything was working as expected. We've deprecated the
ConcurrencyManager as a result. This change swaps out all our usage to
just use the built-in Manager class.

Testing Done

Ran unit tests.

Commits

Summary	ID
Switch out ConcurrencyManager for Manager. Early on in Django history, the built-in `Manager.get_or_create` method was racy, potentially raising `IntegrityError`. We had built a `ConcurrencyManager` to handle this case, and used it for many of our models where we encountered issues. As of quite some time ago, Django's built-in `Manager` does exactly the same thing as our `ConcurrencyManager` did, and we just didn't discover that because everything was working as expected. We've deprecated the ConcurrencyManager as a result. This change swaps out all our usage to just use the built-in `Manager` class. Testing Done: Ran unit tests.	e13897bb1c52d4a9ed5fc38727ad38ea4f6cbb4c

Summary

Switch out ConcurrencyManager for Manager.

Early on in Django history, the built-in `Manager.get_or_create` method was racy, potentially raising `IntegrityError`. We had built a `ConcurrencyManager` to handle this case, and used it for many of our models where we encountered issues. As of quite some time ago, Django's built-in `Manager` does exactly the same thing as our `ConcurrencyManager` did, and we just didn't discover that because everything was working as expected. We've deprecated the ConcurrencyManager as a result. This change swaps out all our usage to just use the built-in `Manager` class. Testing Done: Ran unit tests.

e13897bb1c52d4a9ed5fc38727ad38ea4f6cbb4c

Issues

Description	From	Last Updated
'reviewboard.accounts.models.ReviewRequestVisit' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 4, 2024, 10:57 a.m.
undefined name 'Optional' Column: 21 Error code: F821	reviewbot	Aug. 4, 2024, 10:57 a.m.
'reviewboard.reviews.models.BaseComment' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 4, 2024, 10:57 a.m.
'reviewboard.accounts.models.ReviewRequestVisit' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 4, 2024, 10:57 a.m.
'reviewboard.reviews.models.BaseComment' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 4, 2024, 10:57 a.m.

flake8 failed.

JSHint passed.

flake8

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

The issue has been dropped. Show all issues

'reviewboard.accounts.models.ReviewRequestVisit' imported but unused

Column: 5
Error code: F401

reviewboard/accounts/managers.py (Diff revision 1)
The issue has been dropped. Show all issues
```
undefined name 'Optional'

Column: 21
Error code: F821
```

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

The issue has been dropped. Show all issues

'reviewboard.reviews.models.BaseComment' imported but unused

Column: 5
Error code: F401

Commits:

	Summary	ID
	Switch out ConcurrencyManager for Manager. Early on in Django history, the built-in `Manager.get_or_create` method was racy, potentially raising `IntegrityError`. We had built a `ConcurrencyManager` to handle this case, and used it for many of our models where we encountered issues. As of quite some time ago, Django's built-in `Manager` does exactly the same thing as our `ConcurrencyManager` did, and we just didn't discover that because everything was working as expected. We've deprecated the ConcurrencyManager as a result. This change swaps out all our usage to just use the built-in `Manager` class. Testing Done: Ran unit tests.	29bc543a4a8a9a4380e4b979cbdfb92183b8bde2
	Switch out ConcurrencyManager for Manager. Early on in Django history, the built-in `Manager.get_or_create` method was racy, potentially raising `IntegrityError`. We had built a `ConcurrencyManager` to handle this case, and used it for many of our models where we encountered issues. As of quite some time ago, Django's built-in `Manager` does exactly the same thing as our `ConcurrencyManager` did, and we just didn't discover that because everything was working as expected. We've deprecated the ConcurrencyManager as a result. This change swaps out all our usage to just use the built-in `Manager` class. Testing Done: Ran unit tests.	e13897bb1c52d4a9ed5fc38727ad38ea4f6cbb4c

Diff:

Revision 2 (+80 -42)

Show changes

	reviewboard/accounts/managers.py
	reviewboard/accounts/models.py
	reviewboard/reviews/managers.py
	reviewboard/reviews/models/review_request_draft.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/accounts/managers.py (Diff revision 2)

The issue has been dropped. Show all issues

'reviewboard.accounts.models.ReviewRequestVisit' imported but unused

Column: 5
Error code: F401

reviewboard/reviews/managers.py (Diff revision 2)

The issue has been dropped. Show all issues

'reviewboard.reviews.models.BaseComment' imported but unused

Column: 5
Error code: F401

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (4417e9d)