Conversation
…MAC, and System ID
Codecov Report
@@ Coverage Diff @@
## master #83 +/- ##
==========================================
- Coverage 84.73% 83.46% -1.28%
==========================================
Files 30 31 +1
Lines 2195 2316 +121
Branches 274 297 +23
==========================================
+ Hits 1860 1933 +73
- Misses 249 286 +37
- Partials 86 97 +11 |
| @@ -7,6 +7,7 @@ | |||
| from facility_profile.models import Facility | |||
There was a problem hiding this comment.
Can safely ignore this file. It's primarily just black/linting stuff, and a few small test fixes.
| instance, _ = cls.get_or_create_current_instance() | ||
| cls.objects.filter(id=instance.id).update(counter=F("counter") + 1) | ||
| instance.refresh_from_db(fields=["counter"]) | ||
| return instance |
There was a problem hiding this comment.
These are the changes that should fix the "more than one InstanceIDModel" issue.
| system_id = models.CharField(max_length=100, blank=True) | ||
|
|
||
| @classmethod | ||
| @transaction.atomic |
There was a problem hiding this comment.
The changes in the get_or_create_current_instance method, below, implement the behavior described in #79 (leveraging the helper functions in utils.py)
First, it sees whether the current system parameters (calculated using the "old method") match the "current" instance. If they do, we stick with that to minimize unnecessary churn. If not, we use the "new method" to compute a more robust instance ID.
| @@ -0,0 +1,197 @@ | |||
| import hashlib | |||
There was a problem hiding this comment.
This file is where a lot of the real "work" happens, to support calculation of the Instance ID.
| @@ -11,6 +11,7 @@ | |||
| from facility_profile.models import InteractionLog | |||
There was a problem hiding this comment.
I had to update a number of the tests to change the mocking, since we now compute the Instance ID as a function of different things. This file is an example of that (most of the change is from indentation).
| @@ -1,29 +1,43 @@ | |||
| import hashlib | |||
There was a problem hiding this comment.
Many of the changes here are just from running "black". I'll flag the new stuff.
| InstanceIDModel.objects.exclude(id=ident).filter(current=True).exists() | ||
| ) | ||
| self.assertTrue(InstanceIDModel.objects.get(id=ident).current) | ||
|
|
There was a problem hiding this comment.
The two tests below are new, and are designed to ensure that the Instance ID doesn't change when we aren't wanting it to.
bjester
left a comment
bjester
left a comment
There was a problem hiding this comment.
Only concern would be the frequent usage of # noqa: E722 but it's understandable.
jamalex
force-pushed
the
instance_id_stability
branch
from
27b8b42 to
48bd637
Compare
jamalex
force-pushed
the
instance_id_stability
branch
from
48bd637 to
2157db6
Compare
|
Tests now passing, merging! |
3 participants
Summary
Addresses #79. Instance ID is now calculated as a function purely of the System ID, MAC address, and Device ID. If it wouldn't otherwise have changed, it is left the same, however.
TODO
Reviewer guidance
If you PR has a significant size, give the reviewer some helpful remarks
Issues addressed
List the issues solved or partly solved by the PR
Documentation
If the PR has documentation, link the file here (either .rst in your repo or if built on Read The Docs)