Skip to content

Catch deferred attribute exception#367

Merged
auvipy merged 1 commit intojazzband:masterfrom
slurms:fix-deferred-attributes
Mar 29, 2019
Merged

Catch deferred attribute exception#367
auvipy merged 1 commit intojazzband:masterfrom
slurms:fix-deferred-attributes

Conversation

@slurms
Copy link
Copy Markdown
Contributor

@slurms slurms commented Mar 29, 2019

Problem

Fixes #331.

Solution

Catch the AttributeError and return the underlying field value.

Commandments

  • Write PEP8 compliant code.
  • Cover it with tests.
  • Update CHANGES.rst file to describe the changes, and quote according issue with GH-<issue_number>.
  • Pay attention to backward compatibility, or if it breaks it, explain why.
  • Update documentation (if relevant).

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2019

Codecov Report

Merging #367 into master will decrease coverage by 0.27%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
- Coverage   98.72%   98.45%   -0.28%     
==========================================
  Files           6        6              
  Lines         707      710       +3     
==========================================
+ Hits          698      699       +1     
- Misses          9       11       +2
Impacted Files Coverage Δ
model_utils/tracker.py 96.84% <50%> (-1.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4a252d...959786e. Read the comment docs.

@slurms slurms force-pushed the fix-deferred-attributes branch from 66d3113 to 959786e Compare March 29, 2019 05:38
@jarekwg jarekwg mentioned this pull request May 1, 2019
1 task
@Atorich Atorich mentioned this pull request Aug 20, 2019
mthuurne added a commit to ProtixIT/django-model-utils that referenced this pull request Jun 12, 2024
This catch was introduced in jazzband#367, but subsequent discussion
in jazzband#370, jazzband#381 and jazzband#382 suggests that it wasn't the proper solution.

Now that `AbstractModelTrackerTests` is re-enabled and passing,
it becomes clear that `AttributeError` is no longer raised during
unit testing when using a recent Django version. Therefore I think
it is safe to remove the `try`/`except`.
mthuurne added a commit to ProtixIT/django-model-utils that referenced this pull request Jun 13, 2024
This catch was introduced in jazzband#367, but subsequent discussion
in jazzband#370, jazzband#381 and jazzband#382 suggests that it wasn't the proper solution.

Now that `AbstractModelTrackerTests` is re-enabled and passing,
it becomes clear that `AttributeError` is no longer raised during
unit testing when using a recent Django version. Therefore I think
it is safe to remove the `try`/`except`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants