Conversation
- the category loop read the schema title outside any try, so a category page with no jsonschema slot, no title key, or a non-string title ended load_entity and left the remaining titles unloaded - the loop body is now wrapped in try/except, which logs the page title, the category and the exception, then skips the page like the two error paths that already exist in the same function Closes #202
Contributor
Release previewNo version bump from the current commits (stays at Changelog preview (truncated)Preview via python-semantic-release and conventional commits. |
This branch has not been deployed
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
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.
Closes #202.
Stacked PR
This targets
fix/load-entity-registered-class, the head of#166, not
main. PR #166rewrites the same block, so a branch off
mainwould conflict with it. Merge#166 first, then retarget this one to
main.Review only the single commit
fix(core): skip a page whose category has no usable schema.Changes
for category in jsondata["type"]:inload_entityis wrappedin
try/except Exception.schemas_fetched = False, logs the page title, thecategory and the exception with
_logger.error, and continues.if not schemas_fetched: continuethen skips the page.tests/test_load_entity_bad_category.py.Read the source change with
git diff -w. Most of the 100 changed lines areindentation. Two lines were reflowed by
ruff formatbecause the addedindentation level pushed them past 88 columns.
Rationale
The schema title was read outside any error handling, so three inputs ended
the whole call:
jsonschemaslot, soget_slot_contentreturnsNoneTypeError: 'NoneType' object is not subscriptabletitlekeyKeyError: 'title'titleis present but not a stringTypeError: attribute name must be stringFor a multi-title load this meant the titles after the bad one were never
processed, and the message named neither the page nor the category.
Two error paths in the same function already handle a page-level fault this
way: the missing-model branch sets
schemas_fetched = Falseand logs, and theentity construction below catches
Exception, logs with the page title, andlets the loop continue. The handler added here follows both.
Exceptioniscaught rather than the three specific types, to match the construction block
and to cover any other fault in a category page.
Verification
with
TypeError: 'NoneType' object is not subscriptablefrom the line theissue names.
pytest tests -qon this branch: 674 passed. The count is lower than onmainbecause the base branch does not yet contain the most recentmaincommits.
make check: pre-commit, ruff, ty and deptry all pass.The tests cover the three inputs above, a two-title load where the first page
is broken and the second entity is still returned, and an assertion that the
logged error names both the page and the category.