Add typing to storage/*/migrator.py and storage/sqlite_*/backend.py#7293
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7293 +/- ##
==========================================
+ Coverage 79.85% 79.86% +0.02%
==========================================
Files 566 566
Lines 43947 43962 +15
==========================================
+ Hits 35088 35105 +17
+ Misses 8859 8857 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
danielhollas
force-pushed
the
typing/migrator
branch
from
62bc2d5 to
e7d7ea0
Compare
danielhollas
changed the title
storage/*/migrator.py and storage/sqlite_*/backend.py
|
@GeigerJ2 I found another typing branch that I had locally adding typing the SQlite |
|
|
||
| def maintain(self, dry_run: bool = False, live: bool = True, **kwargs) -> None: | ||
| def maintain(self, full: bool = False, dry_run: bool = False, **kwargs: Any) -> None: | ||
| pass |
There was a problem hiding this comment.
Maybe should raise NotImplemented error? But probably for another PR
There was a problem hiding this comment.
Agreed, out of scope for this PR :)
|
|
||
| def maintain(self, dry_run: bool = False, live: bool = True, **kwargs) -> None: | ||
| def maintain(self, full: bool = False, dry_run: bool = False, **kwargs: Any) -> None: | ||
| pass |
There was a problem hiding this comment.
Agreed, out of scope for this PR :)
|
Thanks, @danielhollas! Will approve once CI has passed :) |
Co-authored-by: Julian Geiger <julian.geiger@psi.ch>
danielhollas
force-pushed
the
typing/migrator
branch
from
ecace73 to
e718b74
Compare
GeigerJ2
left a comment
GeigerJ2
left a comment
There was a problem hiding this comment.
This is great, thanks, as always @danielhollas 🚀
| return {'objects': {'count': len(list(self.list_objects()))}} | ||
|
|
||
| def maintain(self, dry_run: bool = False, live: bool = True, **kwargs) -> None: | ||
| def maintain(self, full: bool = False, dry_run: bool = False, **kwargs: Any) -> None: |
There was a problem hiding this comment.
Nit: This is a repository backend (SandboxShaRepositoryBackend), but the signature uses the storage backend convention (full, dry_run). Other repository backends like DiskObjectStoreRepositoryBackend use (dry_run, live). Both are no-ops here so it doesn't matter, but it's a bit inconsistent.
There was a problem hiding this comment.
Good catch! Fixed it. The type checker did not complain about this because the maintain method is not defined in the AbstractRepositoryBackend base class. So we I think we could either remove it from here, or add it the AbstractRepositoryBackend to enforce consistency. But that's for another PR.
2 participants
Motivated by #7177