Conversation
irtazaakram
commented
Apr 8, 2025
- Resolves Add Django 5.2 support to openedx-learning #296
.annotation_safe_list.yml
Outdated
| auth.User: | ||
| ".. no_pii:": "This model has no PII" |
There was a problem hiding this comment.
Does the User model really have no PII ? And why do we have to declare annotations for common dependencies like Django or waffle redundantly in this repo anyways?!
There was a problem hiding this comment.
The auth.User model does have some PII (username, email, and password), so I’ve added the annotations for it.
I think get_models_requiring_annotations method in find_django.py seems to treats all models (including those from external dependencies) as requiring annotation.
pii_check: commands[0]> code_annotations django_find_annotations --config_file .pii_annotations.yml --lint --report --coverage
Configured for report path: pii_report
Configured for source path: ./
Found safelist at .annotation_safe_list.yml. Reading.
Performing linting checks...
Linting passed without errors.
Model coverage report
----------------------------------------
Found 35 total models.
31 were eligible for annotation, 5 were annotated.
Coverage is 16.1%
Coverage found 26 uncovered models:
auth.User
oel_collections.Collection
oel_collections.CollectionPublishableEntity
oel_components.Component
oel_components.ComponentType
oel_components.ComponentVersion
oel_components.ComponentVersionContent
oel_contents.Content
oel_contents.MediaType
oel_publishing.Container
oel_publishing.ContainerVersion
oel_publishing.Draft
oel_publishing.EntityList
oel_publishing.EntityListRow
oel_publishing.LearningPackage
oel_publishing.PublishLog
oel_publishing.PublishLogRecord
oel_publishing.PublishableEntity
oel_publishing.PublishableEntityVersion
oel_publishing.Published
oel_tagging.ObjectTag
oel_tagging.Tag
oel_tagging.TagImportTask
oel_tagging.Taxonomy
oel_units.Unit
oel_units.UnitVersion
Coverage threshold not met! Needed 100.0, actually 16.1!
| migrations.AlterModelOptions( | ||
| name='entitylistrow', | ||
| options={'ordering': ['order_num']}, | ||
| ), |
There was a problem hiding this comment.
Where is this migration coming from when there's no changes to the model - is it related to Django 5.2 specifically?
There was a problem hiding this comment.
This migration isn't related to Django 5.2 It reflects a change that was made to the entitylistrow model last week, https://github.com/openedx/openedx-learning/pull/292/files#diff-f814bfe5a65c31e277927ed66550877cb86f65b735347d1b478eae5f91825886