Add function to loop over all ports to identify deleted ports #128
Conversation
Codecov Report
@@ Coverage Diff @@
## master #128 +/- ##
==========================================
- Coverage 73.84% 73.31% -0.53%
==========================================
Files 46 46
Lines 1338 1349 +11
==========================================
+ Hits 988 989 +1
- Misses 350 360 +10
Continue to review full report at Codecov.
|
| @classmethod | ||
| def full_deleted_ports_run(cls): | ||
| all_ports_from_json = set() | ||
| data = Port.PortIndexUpdateHandler().sync_and_open_file() |
There was a problem hiding this comment.
I need to check again, but does running this function always fetch the latest version of the json file?
I kind of feel that the functionality should be separate from rsync-ing the file.
There was a problem hiding this comment.
Yes, the function sync_and_open_file first fetches the latest file and then reads it.
|
|
||
| # Loop over all the ports in the database and check if it is available in all_ports_from_json | ||
| with transaction.atomic(): | ||
| for port in Port.objects.all().only('name', 'active'): |
There was a problem hiding this comment.
I would additionally filter out non-active ports. Once you end up with 80% of the ports being deleted, this will make a significant difference.
There was a problem hiding this comment.
Right, thanks. Very fair point.
Yes, and I am not sure how do we trigger this. Maybe a migration? |
Hmmm ... actually migration sounds like a suitable idea (we do need to fix the database, after all). Maybe add a comment next to the function saying that we had to fix the situation once due to buggy code, but that we might want to run that code every now and then. We don't immediately need the perfect solution, and having a function available + running it once might do for now, and then we can think about the best ways to potentially do full updates. I do imagine cases like when we change the format of json to create more user-friendly licence fields. In such cases we'll need to run through the whole database as well. Bottomline: we can merge this before implementing migration (or together, if ready), and worry about the best way to run this later. |
|
We can merge this together with the required migration file. I will add the migration file after we merge #119 just to avoid any conflicts in the migrations. |
|
I am unable to call this model method from the migrations. Seems like we are not allowed to do this unless I move all the code inside the migration file itself. |
|
The new update mechanism has an improved version of this. This PR can be closed. |
This is another part of #122 .
We need to run this function at least once so that the ports that were missed to be marked as deleted get marked correctly. However, I am not sure of the right way to do this.
Any suggestions: @mojca @umeshksingla