Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command to clear snaps in bulk (e.g. last x hours) #5

Open
hirak99 opened this issue Dec 4, 2022 · 7 comments
Open

Command to clear snaps in bulk (e.g. last x hours) #5

hirak99 opened this issue Dec 4, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@hirak99
Copy link
Owner

hirak99 commented Dec 4, 2022

There should be a command to clear snapshots in bulk.

It should have options to specify which tags, and what algorithm should be applied to the clean up.

@hirak99 hirak99 added the enhancement New feature or request label Dec 4, 2022
@hirak99 hirak99 changed the title Commands to clear snaps in bulk (e.g. last x hours) Command to clear snaps in bulk (e.g. last x hours) Dec 11, 2022
@Tql-ws1
Copy link

Tql-ws1 commented Nov 23, 2024

Can you share your thoughts? Maybe I can write this feature out.

It should have options to specify which tags, and what algorithm should be applied to the clean up.

Tags are used for grouping, so they can serve as a filtering option when batch cleaning snapshots, right? And the design of selecting snapshots to clean and the algorithm is intended to use time intervals, such as before or after a certain time, is that correct?

For example:
# Create snapshots with grouping tags
yabsnap create --tag "modify_conf"

# Delete all snapshots with the "modify_conf" tag
yabsnap batch-delete --tag "modify_conf"

# Delete all snapshots with the "modify_conf" tag that belong to the "scheduled" category
yabsnap batch-delete --tag "modify_conf" --indicator "s"

# Delete all snapshots between 2024-11-19 11:38 and 2024-11-21 23:59
yabsnap batch-delete --start "2024-11-19_11:38" --end "2024-11-21"

# Delete snapshots after 2024-11-19 that have the "modify_conf" tag and are in the "scheduled" category
yabsnap batch-delete --start "2024-11-19" --tag "modify_conf" --indicator "S"

# Delete snapshots in the "user" category before 2024-11-21
yabsnap batch-delete --end "2024-11-21" --indicator "u"

Before officially deleting these snapshots, a "preparation for deletion" list of snapshots (deletion range) will be provided for user review.


Chinese version:

可以说说你的想法吗?也许我能把这个功能给写出来。

It should have options to specify which tags, and what algorithm should be applied to the clean up.

标签是用于分组,以便在批量清理快照时作为一个过滤选项,对吗?而挑选出想要清理的快照,挑选算法的设计是打算用时间的区间、XX时间前或XX时间后,是这样吗?

如:
# 创建具有分组标签的快照
yabsnap create --tag "modify_conf"

# 删除所有具有 "modify_conf" 标签的快照
yabsnap batch-delete --tag "modify_conf"

# 删除所有具有 "modify_conf" 标签,且属于 “计划中” 分类的快照
yabsnap batch-delete --tag "modify_conf" --indicator "s"

# 删除 2024-11-19 11:38 至 2024-11-21 23:59 间的所有快照
yabsnap batch-delete --start "2024-11-19_11:38" --end "2024-11-21"

# 删除 2024-11-19 之后,且具有 "modify_conf" 标签、“计划中” 分类的快照
yabsnap batch-delete --start "2024-11-19" --tag "modify_conf" --indicator "S"

# 删除 2024-11-21之前,具有 “用户” 分类的快照
yabsnap batch-delete --end "2024-11-21" --indicator "u"

在正式删除这些快照前,会提供 “准备删除” 的快照列表(删除范围),供用户审阅。

@hirak99
Copy link
Owner Author

hirak99 commented Nov 23, 2024

Yes, that should be useful!

There is no more --tag though. However there are --source and --config-file which should be honored if passed.

@Tql-ws1
Copy link

Tql-ws1 commented Nov 24, 2024

Thank you for your correction and addition. I will get to work on it right away.

@hirak99
Copy link
Owner Author

hirak99 commented Dec 10, 2024

Thank you for the pull request, really nice!
Thank you also for adding additional documentation and improving some of the existing code!

My main request is to move as much of the new functions to a new file as possible. Let's call it batch_deleter.py? This way the functions do not mix up and are easy to find, and it keeps complexity manageable. Also later on this helps to let us add and manage specific tests for batch_deleter (e.g. time conversion, checking against scope, etc.) if needed.

Other than that, I added some other comments, hope they are clear - if anything looks too much work let me know. I can do some of them after you merge as well.

@Tql-ws1
Copy link

Tql-ws1 commented Dec 10, 2024

Move the code for the bulk delete function to a new file? Of course!
batch_deleter.py is a nice name, let's go with it.

I am still pondering whether to write the corresponding unit tests, because I placed part of the new functionality code in snap_operator.py. I have looked at the code in snap_operator_test.py, but I feel that the necessity of adding unit tests is insufficient. At the same time, I am not familiar with the unittest library, and I need some time to learn the relevant knowledge.
But since you brought it up, I will try to implement it.

Actually, before submitting this Pull Request, I wrote a piece of code to separate some responsibilities of the src/code/main.py library and abstract the operations performed by the subcommands, named command_operator.py. However, I realized that this was not within the scope of the current issue, so I ultimately did not submit this file. Below is a code snippet from command_operator.py, which is intended to replace the conditional statements within the _config_operation function in src/code/main.py.

yabsnap_command-operation_code-snippet

@hirak99
Copy link
Owner Author

hirak99 commented Dec 10, 2024

Thank you!

To be clear, your pull request can be merged without unittests - I am happy to do it with manual tests.

In my humble opinion, unit tests help mostly in two things (1) to guard important functionality from breaking as it gets worked on or refactored, so it is critical to have it especially for a section of the code that is complicated, and will undergo further change; and (2) as a guarantee that the code does what we want; e.g. for test-driven development, or for fixing bugs (where a test may be written to prove that the code operates in the desired manner), or simply for documentation and easier understanding of code behavior.

Having said that if you would like you are welcome to write them, now or as another PR to test critical functionalities. Of course it will only improve the code health, and we will merge that in as well.

@Tql-ws1
Copy link

Tql-ws1 commented Dec 11, 2024

No problem, please give me some time to understand the relevant knowledge about unit testing and the unittest library. After that, I will submit the unit test code related to it, as well as the code that splits the new functionality into other files as mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants