DM-33634: Add pipetask purge and cleanup subcommands #178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
n8pease
force-pushed
the
tickets/DM-33634
branch
2 times, most recently
from
874e10a
to
cbf395f
Compare
n8pease
force-pushed
the
tickets/DM-33634
branch
from
cbf395f
to
e154e9f
Compare
Codecov Report
@@ Coverage Diff @@
## main #178 +/- ##
==========================================
+ Coverage 81.38% 82.55% +1.16%
==========================================
Files 41 47 +6
Lines 3298 3628 +330
Branches 550 588 +38
==========================================
+ Hits 2684 2995 +311
- Misses 454 465 +11
- Partials 160 168 +8
Continue to review full report at Codecov.
|
TallJimbo
reviewed
Apr 6, 2022
| butler_config : str | ||
| The path location of the gen3 butler/registry config file. | ||
| collection : str | ||
| TODO |
| self.type = type | ||
|
|
||
| def __str__(self): | ||
| return f'COLLETION must be a CHAINED collection, "{self.collection}" is a "{self.type}" collection.' |
| butler = Butler(self.butler_config, writeable=True) | ||
| for collection in self.others_to_remove: | ||
| butler.registry.removeCollection(collection) | ||
| butler.removeRuns(self.runs_to_remove) |
There was a problem hiding this comment.
Probably worth wrapping this line and the for loop above in with butler.transaction() so it's all atomic.
| """ | ||
| pass | ||
|
|
||
| @abstractmethod |
There was a problem hiding this comment.
I believe
@property
@abstractmethod
is now preferred.
|
|
||
| def __str__(self): | ||
| parents = ", ".join([f'"{p}"' for p in self.parents]) | ||
| return f'Collection "{self.child}" is in multiple chained collections: {parents}.\n' + advice |
There was a problem hiding this comment.
Since you've already got an f-string going, just interpolate advice in, too.
| def __init__(self, butler_config: str): | ||
| self.runs_to_remove = [] | ||
| self.chains_to_remove = [] | ||
| self.others_to_remove = [] |
There was a problem hiding this comment.
Making these sets instead of lists seems like would reflect the usage better, unless you're concerned about printing them for confirmation in a way that maintains order.
| purge_result : PurgeResult | ||
| The description of what datasets to remove and/or failures encountered | ||
| while preparing to remove datasets to remove, and a completion function | ||
| to remove the datests after confirmation, if needed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
doc/changes