Skip to content
This repository was archived by the owner on Feb 3, 2025. It is now read-only.

Added local Repo Support #106

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

Conversation

Saksham1387
Copy link
Contributor

What does this PR do?

It enable users to chat with their Repo's instead of pulling them from Github.

To index now this is what the CLI command should look like:
Screenshot 2024-11-08 at 11 58 30

and this is the chat command:
Screenshot 2024-11-08 at 11 58 40

Fixes # (issue)
#103

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Was this discussed/approved via a Github issue? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who
may be interested in your PR.

Copy link
Contributor

@iuliaturc iuliaturc left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
I left a suggestion around preventing duplication.
Additionally, could you please run these formatting commands:

isort --profile=black .
black --line-length=120 .

sage/chat.py Outdated
@@ -74,6 +73,7 @@ def main():

validator = sage_config.add_all_args(parser)
args = parser.parse_args()
print(args)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: could you please remove the print statement?

@@ -81,6 +81,11 @@ def add_repo_args(parser: ArgumentParser) -> Callable:
default="repos",
help="The local directory to store the repository",
)
parser.add(
"--repo-mode",
default = "remote",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also define the possible values (e.g. choices=["local", "remote"])

Comment on lines 258 to 274
def from_args_localRepo(args: Dict):
"""Creates a GitHubRepoManager from command-line arguments and clones the underlying repository."""
repo_manager = GitHubRepoManager(
repo_id=args.repo_id,
commit_hash=args.commit_hash,
access_token=os.getenv("GITHUB_TOKEN"),
local_dir=args.local_dir,
inclusion_file=args.include,
exclusion_file=args.exclude,
)
if not repo_manager.local_dir:
# Proceed if local_dir exists and is not empty
raise ValueError(
f"The given Directory is empty, please provide the correct local dir "
)

return repo_manager
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a new from_args_localRepo method (which introduces a lot of duplication with from_args), I would simply change the implementation of repo_manager.download() to only call git clone if the repository doesn't already exist locally.

@Saksham1387
Copy link
Contributor Author

I have made the required changes you mentioned, please have a look. @iuliaturc

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

Successfully merging this pull request may close these issues.

2 participants