API: Various small fixes + linting - #1256
Open
peterthomassen wants to merge 16 commits into
Open
Conversation
When another request deletes an RR set between the replacement of its RRs and the subsequent save(), Django re-inserts the row instead of updating it, and the tracker sees a creation for an RR set already recorded as modified. Raise ConcurrencyException there instead of failing an assertion, so that the transaction is rolled back, no changes are sent to pdns, and the client gets a 429 instead of a 500.
PermissionValidator no longer assumes that the view has a `subname` URL kwarg or that the serializer has an instance. On the `rrsets/` bulk endpoint, an item with `records: []` that omits `subname` or `type` is validated as partial, so neither field reaches the validator; those items are now passed on to UniqueTogetherValidator, which reports the missing field. The docs are updated to match: `subname` is required in bulk requests since a34b963 (2023) and does not default to the zone apex.
AccountLogoutView.get_object() returns the token that authenticated the request instead of looking it up by key a second time. Concurrent logouts of the same token now delete an already-deleted row and answer 204, rather than raising Token.DoesNotExist after the first one wins.
replace_ip_subnet() builds the result on top of the subnet's network address instead of round-tripping it through ip_address(int(...)), which returned an IPv4Address for every IPv6 subnet whose network address fits in 32 bits (such as ::/0, ::/56 and ::/64) and then raised AddressValueError when the host bits of an AAAA record were added to it.
test_subnet now gives foo both an A and an AAAA RRset and replaces each with its own subnet, so that replace_ip_subnet()'s address family filter is exercised with input holding records of both families.
RR.canonical_presentation_format() now parses its own output once more and raises ValueError when that fails, so that content dnspython renders but cannot read back (such as an SVCB or HTTPS record with an empty parameter value, given as key1="") is answered with 400 instead of crashing the uniqueness check that converts the canonical form to wire format.
DynDNS12UpdateView.qnames() treats an account that owns no domain like one that owns several: when the request does not name a domain and none can be inferred, it raises the "domain-unspecified" ValidationError instead of letting Domain.DoesNotExist escape.
The API basics now say that every endpoint URL carries a trailing slash, placed immediately before the query string where one is given.
The RRsetDetail endpoint takes the subname from the URL, so it is absent from the serializer's validated data on PATCH, and the length estimate was computed against the apex qname. Fall back to the instance's subname.
The zone creation and deletion callbacks compared the raw request body against a dict, and the resulting assertion error was discarded by the enclosing `finally: return`, so the body was never checked. Match the body through `responses` instead, which also pairs each expectation with the right request when several PCH requests are made.
Replace printf-style formatting and `str.format()` calls with f-strings where the result is equivalent, drop the deprecated `typing.Tuple`/`typing.Callable` imports, and use `datetime.UTC`.
Apply isort ordering (standard library, third party, first party, local), which also removes a duplicate import of `RR_SET_TYPES_UNSUPPORTED`.
Drop imports that are not referenced, stray `f` prefixes on strings without placeholders, and `BASE_DIR`, which Django does not use by itself and which no setting refers to.
Catch `Exception` instead of using a bare `except`, pass `strict=True` to `zip()` where the operands are known to be of equal length, use `isinstance()` for a type check, and rename loop and argument names that shadow their iterable or are hard to tell apart from digits.
Enable ruff's pyflakes, pyupgrade, isort, bugbear and the non-formatting pycodestyle rules in api/ruff.toml, run `ruff check api/` in CI next to the formatting check, and pin the ruff version so that a new release does not turn an unrelated pull request red. Suppressions are attached to the sites they apply to. Closes #1251
Drop `desecapi/pch.py` and the `pch_do` hook that `PDNSChangeTracker` called on domain creation and deletion, along with `PCHException`, the two PCH metrics, the `DESECSTACK_API_PCH_API` and `DESECSTACK_API_PCH_API_TOKEN` variables, and the PCH request expectations in the test base class. `ExternalAPIException` is folded into `PDNSException`, which was its only remaining subclass. The now unused `assertZoneCreation()` and `assertZoneDeletion()` helpers go with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.