Fix for Issue #54 #81
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
mlc/main.py
Outdated
| if res["return"] > 0: | ||
| return res | ||
| logger.warning(f"{meta_yaml_path} not found. Could be due to accidental deletion of meta.yaml. Try to stash the changes or reclone by doing `rm repo` and `pull repo`. Skipping...") | ||
| # logger.warning(f"Deleting the {meta_yaml_path} entry from repos.json") |
There was a problem hiding this comment.
@arjunsuresh , I have commented out this part as I'm not completely sure whether automatically deleting the repo is a good choice here. The repo is already registered, that means there should have been a meta.yaml in the beginning. So maybe one has accidentally deleted the yaml file or has checkout out to a branch in the same repository which does not have the meta.yml file.
There was a problem hiding this comment.
In rm repo we are not physically deleting the repository right? In that case it is fine. Meanwhile it'll be good to add an explicit option for physically deleting the repository.
There was a problem hiding this comment.
Currently the rm repo does the following things:
- Deletes the repo folder if its present
- Deletes the repo entry from repos.json
There was a problem hiding this comment.
Thanks Anandhu. So the commented code can be removed?
There was a problem hiding this comment.
I think its safe to remove. Have made changes in latest commit
| - name: Test 21 - Test autoremoval of corrupt entries from repos.json while running a script | ||
| run: | | ||
| rm -r $HOME/MLC/repos/mlcommons@mlperf-automations | ||
| mlc run script --tags=detect,os |
There was a problem hiding this comment.
script automation won't exist after the previous removal right?
There was a problem hiding this comment.
My bad :)
In the latest commit, a new corrupt entry has been added manually using jq library.
| @@ -195,3 +195,9 @@ jobs: | |||
| mlc list cache | |||
|
|
|||
|
|
|||
| - name: Test 21 - Test autoremoval of corrupt entries from repos.json while running a script | |||
| run: | | |||
| sudo apt-get install -y jq | |||
There was a problem hiding this comment.
Currently the test is covering only ubuntu. When I think about this, I'm getting four ways to proceed with this test.
- We could create a sample MLC repo , clone it and manually remove it which creates a corrupt entry.
- We could create a corrupt entry in the current way by using system libraries, but we might have to handle the os specific conditions.
- We could create a python file and simply call that file to modify the repos entry.
- We could create a helper function in mlc itself to proceed with this, but ig that would need some work.
There was a problem hiding this comment.
Removed test 21 as per latest discussion. Could add in future if needed
No description provided.