feat: support batch delete snapshots #38
Conversation
| def _convert_to_timestamp(suffix: str) -> str: | ||
| """Convert human-readable time format into a timestamp.""" | ||
| if not _human_friendly_timestamp(suffix): | ||
| return suffix | ||
|
|
||
| try: | ||
| # "2024-11-19_15:30".split("_") -> ["2024-11-19", "15:30"] | ||
| date, time = suffix.split("_") | ||
| except ValueError: # unpack error | ||
| date = suffix | ||
| # If the user does not provide the hour and minute, it defaults to 23:59 | ||
| hour_minute = "2359" | ||
| else: | ||
| # "15:30".split(":") -> ["15", "30"] -> "1530" | ||
| # "1530".split(":") -> ["1530"] -> "1530" | ||
| hour_minute = "".join(time.split(":")) | ||
| suffix_not_minutes = len(hour_minute) == 2 | ||
| if suffix_not_minutes: | ||
| hour_minute += "00" | ||
|
|
||
| days_not_padded_zeros = r"^\d{4}-\d{2}-\d$" | ||
| pad_days_with_zeros = r"^\d{4}-\d{2}-0\d$" | ||
| year_month_day = re.sub(days_not_padded_zeros, pad_days_with_zeros, date) | ||
| return year_month_day + hour_minute |
There was a problem hiding this comment.
Premise
Assume there is 1 configuration and 5 snapshots:
Config: xxx/y.conf
Snaps:
1 202412131055 U
2 202412132359 S
3 202412140810 U
4 202412142359 S
5 202412151756 UUsing --start TIMESTAMP, --end TIMESTAMP, and --start TIMESTAMP1 --end TIMESTAMP2:
- It should delete all snapshots, but actually deletes snapshot number
2and all subsequent snapshots.yabsnap batch-delete --start 2024-12-13(_23:59)
The ideal timestamp completion should be:yabsnap batch-delete --start 2024-12-13(_00:00)
- It should delete snapshots before number
4, and it actually deletes snapshots before number4(the timestamp completion here is correct).yabsnap batch-delete --end 2024-12-14(_23:59)
- It should delete snapshots
1-3, but actually deletes2and3.yabsnap batch-delete --start 2024-12-13 --end 2024-12-14
This is equivalent to:yabsnap batch-delete --start 2024-12-13_23:59 --end 2024-12-14_23:59
The ideal completion for the above shorthand dates should be:yabsnap batch-delete --start 2024-12-13_00:00 --end 2024-12-14_23:59
Issue
After reviewing the above examples, I believe you can understand that the current _convert_to_timestamp function has flaws.
It has the following issues:
- When the timestamp provided by the user has a precision of at most days (e.g.
2024-01-01), the timestamp completion is inaccurate. - The design for zero-padding of months and days, as well as the completion of minutes, is not well-implemented (the following are simplified examples):
2024-5-01->2024-05-012024-01-5->2024-01-052024-01-01_02->2024-01-01_02:00
Thoughts
When writing the _convert_to_timestamp function, I did not consider the introduction of state. When the timestamp provided by the user has a precision of at most days, it is necessary to know which command-line optional parameters (--start, --end) the user has used so that the _convert_to_timestamp function can provide accurate timestamp completion.
Currently, regardless of which optional parameter (--start, --end) is used, the timestamp completion is always _23:59, which is unreasonable. However, introducing state would increase the complexity of the code, which exceeds my expectations.
My thought is to abandon the implementation of "diverse human-friendly timestamp parameters" and instead limit users to only input specific formats of timestamps, raising the upper limit to reduce the complexity of functionality implementation and avoid unintentional invalid input from users. What are your thoughts and suggestions on this?
#5