Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 70 additions & 19 deletions mlc/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ def is_curdir_inside_path(base_path):
self.local_repo = (meta['alias'], meta['uid'])
# Create a Repo object and add it to the list
repos_list.append(Repo(path=repo_path, meta=meta))

return repos_list

def load_repos(self):
Expand Down Expand Up @@ -1007,20 +1006,73 @@ def unregister_repo(self, repo_path):
return {'return': 0}

def find(self, run_args):
repo = run_args.get('item', run_args.get('artifact'))
repo_split = repo.split(",")
if len(repo_split) > 1:
repo_uid = repo_split[1]
repo_name = repo_split[0]

lst = []
for i in self.repos:
if repo_uid and i.meta['uid'] == repo_uid:
lst.append(i)
elif repo_name == i.meta['alias']:
lst.append(i)
try:
# Get repos_list using the existing method
repos_list = self.load_repos_and_meta()
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't the load_repos_and_meta be automatically called when initialising the Action class while running mlc? I think it could be accessed through self.repos_list .

if(run_args.get('item', run_args.get('artifact'))):
Copy link
Contributor

@anandhu-eng anandhu-eng Feb 8, 2025

Choose a reason for hiding this comment

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

Do we need an if condition here since repo, item, and artifact are being considered under else?

repo = run_args.get('item', run_args.get('artifact'))
else:
repo = run_args.get('repo', run_args.get('item', run_args.get('artifact')))

# Check if repo is None or empty
if not repo:
return {"return": 1, "error": "Please enter a Repo Alias, Repo UID, or Repo URL in one of the following formats:\n"
"- <repo_owner>@<repos_name>\n"
"- <repo_url>\n"
"- <repo_uid>\n"
"- <repo_alias>\n"
"- <repo_alias>,<repo_uid>"}

# Handle the different repo input formats
repo_name = None
repo_uid = None

# Check if the repo is in the format of a repo UID (alphanumeric string)
if self.is_uid(repo):
repo_uid = repo
if "," in repo:
repo_split = repo.split(",")
repo_name = repo_split[0]
if len(repo_split) > 1:
repo_uid = repo_split[1]
elif "@" in repo:
repo_name = repo
elif "github.com" in repo:
result = self.github_url_to_user_repo_format(repo)
if result["return"] == 0:
repo_name = result["value"]
else:
return result

# Check if repo_name exists in repos.json
matched_repo_path = None
for repo_obj in repos_list:
if repo_name and repo_name == os.path.basename(repo_obj.path) :
matched_repo_path = repo_obj
break

# Search through self.repos for matching repos
lst = []
for i in self.repos:
if repo_uid and i.meta['uid'] == repo_uid:
lst.append(i)
elif repo_name == i.meta['alias']:
lst.append(i)
elif self.is_uid(repo) and not any(i.meta['uid'] == repo_uid for i in self.repos):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this looping create un-necessary complexity as we are already inside a loop which tries to do the same function in more generic way?

return {"return": 1, "error": f"No repository with UID: '{repo_uid}' was found"}
elif "," in repo and not matched_repo_path and not any(i.meta['uid'] == repo_uid for i in self.repos) and not any(i.meta['alias'] == repo_name for i in self.repos):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. I think usage of any could reduce the number of lines but with a different logic.

@Sid9993 do you think the code would be more efficient and small if we use the below snippet with revised logic?

matching_repos = [i for i in self.repos if i.meta['uid'] == repo_uid]

return {"return": 1, "error": f"No repository with alias: '{repo_name}' and UID: '{repo_uid}' was found"}
elif not matched_repo_path and not any(i.meta['alias'] == repo_name for i in self.repos) and not any(i.meta['uid'] == repo_uid for i in self.repos ):
return {"return": 1, "error": f"No repository with alias: '{repo_name}' was found"}

return {'return': 0, 'list': lst}
# Append the matched repo path
if(len(lst)==0):
lst.append(matched_repo_path)

return {'return': 0, 'list': lst}
except Exception as e:
# Return error message if any exception occurs
return {"return": 1, "error": str(e)}

def github_url_to_user_repo_format(self, url):
"""
Expand Down Expand Up @@ -1462,14 +1514,14 @@ def mlcr():

def process_console_output(res, target, action, run_args):
if action == "find":
if "list" not in res:
return # Exit function if there's an error
Copy link
Contributor

Choose a reason for hiding this comment

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

The check for key list is nice to have, while returning, we usually follow the pattern below for error:

return {"return": error-code, "error": detail-of-error}

if len(res['list']) == 0:
logger.warn(f"""No {target} entry found for the specified tags: {run_args['tags']}!""")
else:
for item in res['list']:
logger.info(f"""Item path: {item.path}""")



# Main CLI function
def main():
parser = argparse.ArgumentParser(prog='mlc', description='A CLI tool for managing repos, scripts, and caches.')
Expand Down Expand Up @@ -1511,9 +1563,8 @@ def main():
run_args = res['args_dict']
if hasattr(args, 'repo') and args.repo:
run_args['repo'] = args.repo


if args.command in ['pull', 'rm', 'add']:

if args.command in ['pull', 'rm', 'add', 'find']:
if args.target == "repo":
run_args['repo'] = args.details

Expand Down
Loading