Track unused models and automatically delete them : issue-08

[Amna-28]

Hi @miquelduranfrigola,

on issue #8 , I have come up with the following solution:

• Changes in fetch.py : "fetched_models.txt" maintains every model that is fetched. It stores model name with timestamp when it
  was fetched (initially)
• Changes in serve.py : whenever a model is run, file "fetched_models.txt" gets modified to update the last usage timestamp,
  whenever ersilia hub is used, i.e. when serve.py is executed, it checks if there is any model on the disk unused for more than x
  time (I have set it to 5 minutes just for testing) and calls delete command for all unused models.
• Changes in delete.py : added the code that deletes the entry of a model from "fetched_models.txt" when a model gets
  deleted

Please guide me for the following queries :

1. I could not figure out how delete functions could be invoked from serve.py, I have used the cli command using os.system for the time being.
2. I am not sure if the deletion being carried out in the serve.py, on every invokation of serve command, is the best solution. Is there any way that the deletion script runs automatically after fixed interval?

The required functionality is running fine overall, but I think there must be a better approach, I would love to hear your feedback on this

[miquelduranfrigola]

Hi @Amna-28 this is a very good approach, many thanks for taking the lead.

I agree with you: deletion of other models should not be done inside the serve command. But having said that, I think you are very close to a good solution - I like the fetched_models.txt file. Now that we have it, perhaps we can do the following:

1. We create a last_cleaned.json file, containing a timestamp corresponding to the last time a systematic deletion of unused models was done.
2. In the master __init__.py of ersilia, we check the last_cleaned.json file:

• If the timestamp is older than, let's say, one week, we trigger a systematic deletion. This systematic deletion simply checks the fetched_models.txt and deletes the ones that have timestamps older than (let's say) one week. Then, the last_cleaned.json is updated with the current timestamp.
• If the timestamp is not older than one week, then we do nothing.

What do you think about this approach?

[Amna-28]

Hi @miquelduranfrigola , this i a great approach I think
Now what I have to do is :

• in init.py : perform the check for last cleaned using last_cleaned.json file, if it is older than a week, we invoke the deletion using fetched_models.txt
• in serve.py : update timestamp in fetched_models.txt

I will do the changes shortly, thanks for your guidance.

[miquelduranfrigola]

This sounds very good! Just keep the "one week" in a variable so we can modify it easily. All the rest looks great.

[Amna-28]

This sounds very good! Just keep the "one week" in a variable so we can modify it easily. All the rest looks great.

Just read your comment, let me add this to a variable.

[miquelduranfrigola]

Hi @Amna-28 I am ready to accept your pull request. Before doing that, I am wondering if your code should go in the CLI submodule (as it does now), or rather be applicable to CLI-independent usage (e.g. as a python library).

It should be relatively straightforward to migrate your code to the python classes themselves (ModelFetcher, ModelDeleter...) and to the main init.py file.

What do you think? Some people use Ersilia as a python package (not as a CLI) and sadly your nice functionalities will not run in their cases.

Happy to gather your feedback. If you want to keep the code like this for the moment, I think it is perfectly fine too and I am ready to do the merging.

[Amna-28]

Hi @Amna-28 I am ready to accept your pull request. Before doing that, I am wondering if your code should go in the CLI submodule (as it does now), or rather be applicable to CLI-independent usage (e.g. as a python library).

It should be relatively straightforward to migrate your code to the python classes themselves (ModelFetcher, ModelDeleter...) and to the main init.py file.

What do you think? Some people use Ersilia as a python package (not as a CLI) and sadly your nice functionalities will not run in their cases.

Happy to gather your feedback. If you want to keep the code like this for the moment, I think it is perfectly fine too and I am ready to do the merging.

Thank you for your feedback @miquelduranfrigola ,
I agree with you, adding the code as python classes would be more appropriate approach and I would be happy to do it in a systematic way. I will start looking into the code but I will need some guidance from you as I do not fully understand the full code structure of the library.

[miquelduranfrigola]

Hi @Amna-28, of course, happy to help.

For starters, how about creating a ersilia/utils/chron.py file where you place your functions and/or classes?

For example, you can have a function called clean_unused_models, a function called update_model_fetch_time, delete_unused_model, etc.

Once all functions are created, we can figure out where to place them exactly.

Sounds good?

[Amna-28]

Hi @miquelduranfrigola , that sounds great. I will get back to you once I create the functions in the ersilia/utils/chron.py file

[Riyabelle25]

Really nice work with the tracking of unused models @Amna-28 😍. I see you're now shifting your code into ersilia/utils/chron.py
Perhaps I could write the delete_unused_model or clean_unused_model functions (using ModelFullDeleter as @miquelduranfrigola suggested just now, rather than os.system as has been used currently) from your PR itself?

[Amna-28]

Really nice work with the tracking of unused models @Amna-28 😍. I see you're now shifting your code into ersilia/utils/chron.py Perhaps I could write the delete_unused_model or clean_unused_model functions (using ModelFullDeleter as @miquelduranfrigola suggested just now, rather than os.system as has been used currently) from your PR itself?

Hi @Riyabelle25 , thankyou for your appreciation and interest.
Sure, let's do this together

[Riyabelle25]

Updates:
I've now migrated @Amna-28's code into pythonic methods under utils/cron.py, and have committed these changes in a PR at her fork 🌸
She will be merging it soon with the ongoing PR here

[Amna-28]

Hi @Riyabelle25 , many thanks for helping me and migrating the code. I am merging your code into my branch
Hi @miquelduranfrigola , please have a look at the merged code and guide us further.

[miquelduranfrigola]

Hi @Riyabelle25 and @Amna-28 really appreciate you cooperating here. Thanks.

All of this is really great stuff.

I really like to keep everything under cli slim in terms of code, meaning I try to embed all code in the python classes (ModelFetcher, ModelFullDeleter, etc.), not in the CLI submodule. The reason for this is that we want people to use ersilia as a python library, a CLI or, in the future, as a GUI, indistinctly. So introducing code specifically to CLI will cause some functionalities to only work in this usage mode.

In the current case, if we use ersilia as a Python library, the cron jobs will not be executed, which is a pity :) Do you think we would migrate the bulk of the code to cron.py, and then just import the functions correspondingly, for example, in the ModelFetcher, ModelFullDeleter, etc?

I know this is complicated, so please do not spend too much extra time on this if you get stuck. I am mindful of your time.

[Amna-28]

Hi @miquelduranfrigola , I get your point.
I will transfer everything to the cron.py funtions and then we will discuss where to use those functions.

[Amna-28]

Hi @miquelduranfrigola , I made the following changes:

• created a method _fetchtime() in ModelFetcher class, that records the fetch timestamp for each model in fetched_models.txt. It is being called in fetch() method of the same class.
• similarly, created delete_model_entry() method in ModelFullDeleter class. When delete operation is executed , this function deletes the model's entry from fetched_models.txt.
• update_model_usage_time() method is created in ErsiliaModel class, which is being called in serve() method, so that the timestamp of the model gets updated whenever serve() is called.
• All the code from fetch, delete, serve and init under the commands directory has been cleared out.
• cron.py contains function model_cleanup() that uses last_cleaned.json and fetched_models.txt to initiate the deletion of unused models as done by @Riyabelle25
  For the time being I have manually run the model_cleanup() function to verify and it seems fine to me. We can call the funtion in serve() method of ErsiliaModel class or wherever you suggest. Please correct me in case of any mistake or if there is anything that I missed.

[miquelduranfrigola]

This looks very good. Approving PR!

[miquelduranfrigola]

Hi @Amna-28 this looks great. I have approved the PR hoping CircleCI tests will pass. I will let you know if we find any issue, but this looks great overall!