Skip to content

feat(model): Implement safe model deletion with trash functionality #283

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SyedaAnshrahGillani
Copy link

This pull request introduces a new "trash" feature to the ART framework,
providing a safety net for model deletion. Previously, deleting a model was
a destructive and irreversible operation. Now, models are moved to a .trash
directory, allowing for recovery or permanent deletion at a later time.

This PR includes the following changes:

  • Safe Deletion: The delete_model function in src/art/model.py now moves
    models to a .trash directory instead of permanently deleting them.
  • Trash Management: New functions have been added to src/art/model.py to
    manage the trash:
    • list_trash: Lists all models in the trash.
    • restore_from_trash: Restores a model from the trash.
    • empty_trash: Permanently deletes all models from the trash.
  • CLI Integration: The new trash functionality is exposed through the CLI in
    src/art/cli.py with the following commands:
    • delete: Moves a model to the trash.
    • list-trashed-models: Lists all models in the trash.
    • restore-model: Restores a model from the trash.
    • empty-trashed-models: Permanently deletes all models from the trash.

This feature significantly improves the safety and usability of the ART
framework by preventing accidental data loss.

@Mirza-Samad-Ahmed-Baig
Copy link

Good Catches.

Copy link
Contributor

@arcticfly arcticfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! How does this apply to the model.delete_checkpoints method? I can imagine a backup of old checkpoints that isn't automatically synced to s3 being useful.

@SyedaAnshrahGillani SyedaAnshrahGillani force-pushed the feature/safe-model-deletion branch from f0d7cec to 2335282 Compare August 11, 2025 08:47
@SyedaAnshrahGillani
Copy link
Author

Nice! How does this apply to the model.delete_checkpoints method? I can imagine a backup of old checkpoints that isn't automatically synced to s3 being useful.

That's a great question,

You're right, the current implementation of the trash feature is focused on
the model as a whole and doesn't extend to the delete_checkpoints method.
The delete_checkpoints function still deletes the checkpoints permanently
from the local filesystem.

I think applying a similar "trash" concept to checkpoints is an excellent
idea for a future enhancement. It would provide a safety net for local
checkpoints that haven't been synced to S3. I can create a new issue to
track this feature.

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

Successfully merging this pull request may close these issues.

3 participants