use dictionary instead of list for Controller.state['backups'] #175
Conversation
| @@ -75,6 +75,26 @@ class BaseBackupFailureReason(str, enum.Enum): | |||
| xtrabackup_error = "xtrabackup_error" | |||
|
|
|||
|
|
|||
| class BaseBackup(TypedDict, total=False): | |||
There was a problem hiding this comment.
Just moved this from myhoard.Controller and added rest of expected fields. The total=False is mainly in case a basebackup.json file is not containing all keys (ideally shouldn't happen but just in case)
| backups = [] | ||
| for site_name, site_config in backup_sites.items(): | ||
| file_storage = site_transfers.get(site_name) | ||
| def get_backups(self) -> Dict[str, Backup]: |
There was a problem hiding this comment.
Removed previous arguments... 🤷♀️ we were just passing class properties that can be accessed here and it was quite odd.
| @@ -75,7 +75,7 @@ async def backup_list(self, _request): | |||
| } | |||
| with self.controller.lock: | |||
| if self.controller.state["backups_fetched_at"]: | |||
| response["backups"] = self.controller.state["backups"] | |||
| response["backups"] = list(self.controller.state["backups"].values()) | |||
There was a problem hiding this comment.
comment left as a list here, I don't see a reason for changing the response type
Codecov Report
@@ Coverage Diff @@
## master #175 +/- ##
==========================================
+ Coverage 78.93% 79.57% +0.64%
==========================================
Files 17 17
Lines 4400 4396 -4
Branches 995 983 -12
==========================================
+ Hits 3473 3498 +25
+ Misses 693 667 -26
+ Partials 234 231 -3
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I don't know if we have any policy related to backward compatibility and probably earlier changes have been incompatible too (perhaps to lesser extent) but if anyone is doing in-place upgrades for MyHoard then it will break when upgrading from a version prior to this one. |
Oh yes, this will definitely break for in-place upgrades 😬 tho not sure if working on backward compatibility is worth it, old state file should be just removed |
kathia-barahona
force-pushed
the
typing_controller_state_backups
branch
from
e395ab7
to
818862e
Compare
kathia-barahona
force-pushed
the
typing_controller_state_backups
branch
from
818862e
to
8da07ed
Compare
kathia-barahona
force-pushed
the
typing_controller_state_backups
branch
from
8da07ed
to
775ed44
Compare
About this change: What it does, why it matters
Changed the way we store backups in the controller's state, main reason is because there a lot of scenarios where we need to fetch a backup based on its stream id. Also added more typing when backups are refreshed.
Not super urgent, but will be nice to have.