Skip to content

Comments

Added revision parameter for results repo#3764

Merged
Samoed merged 3 commits intoembeddings-benchmark:mainfrom
ayush1298:add_revision_to_download_results
Dec 18, 2025
Merged

Added revision parameter for results repo#3764
Samoed merged 3 commits intoembeddings-benchmark:mainfrom
ayush1298:add_revision_to_download_results

Conversation

@ayush1298
Copy link
Collaborator

Closes #3179

Copilot AI review requested due to automatic review settings December 17, 2025 19:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a revision parameter to the download_from_remote method in the ResultCache class, allowing users to specify a specific branch, tag, or commit to checkout from the results repository.

Key changes:

  • Added revision parameter to download_from_remote method signature and documentation
  • Modified git operations to support checking out specific revisions when cloning or updating the repository
  • Updated fetch logic to use git fetch --all --tags instead of git pull when a revision is specified

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ayush1298
Copy link
Collaborator Author

ayush1298 commented Dec 18, 2025

@Samoed, I have removed the depth 1 thing and then tested it with below code:

from mteb.cache import ResultCache

cache = ResultCache()
cache.download_from_remote(revision="cff1581855eb0a6f0d6a96faa1e0e4bc61033e28")

but I am getting below error:

CalledProcessError: Command '['git', 'clone', '--revision=cff1581855eb0a6f0d6a96faa1e0e4bc61033e28', 'https://github.com/embeddings-benchmark/results', 'remote']' returned non-zero exit status 129.

I think issue was with my git version, because when I have tested command:

git clone --revision=cff1581855eb0a6f0d6a96faa1e0e4bc61033e28 https://github.com/embeddings-benchmark/results remote

it works perfectly fine and I have also confirmed it from logs that correct revision is cloned

@Samoed
Copy link
Member

Samoed commented Dec 18, 2025

it works perfectly fine and I have also confirmed it from logs that correct revision is cloned

Yes, this command is working for me too, even with --depth 1

@ayush1298
Copy link
Collaborator Author

it works perfectly fine and I have also confirmed it from logs that correct revision is cloned

Yes, this command is working for me too, even with --depth 1

I think removing --depth 1 will be better.

Anything other in this PR?

@Samoed
Copy link
Member

Samoed commented Dec 18, 2025

I think removing --depth 1 will be better.

You shoudn't remove it

@ayush1298
Copy link
Collaborator Author

ayush1298 commented Dec 18, 2025

Using --depth 1 with --branch for a specific commit SHA may fail because shallow clones only fetch the tip of branches. If the revision parameter can be a commit SHA (not just a branch or tag), consider either removing --depth 1 when a revision is specified, or handle the case where the commit is not reachable in a shallow clone. The current implementation will work for branches and tags but may fail for arbitrary commit SHAs.

Why so? I think as copilot mentioned it will be better to remove it

@Samoed
Copy link
Member

Samoed commented Dec 18, 2025

Why so? I think as copilot mentioned it will be better to remove it

With it downloading repo will be much faster. Also, you don't use branch argument, so copilot comment is irrelevant

@ayush1298
Copy link
Collaborator Author

With it downloading repo will be much faster. Also, you don't use branch argument, so copilot comment is irrelevant

Added it again.

Copy link
Contributor

@KennethEnevoldsen KennethEnevoldsen left a comment

Choose a reason for hiding this comment

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

This looks good to me - thanks for the change! Unsure if we want to add some tests here though

@Samoed any remaining concerns?

@Samoed Samoed merged commit e8c02b1 into embeddings-benchmark:main Dec 18, 2025
9 checks passed
@ayush1298 ayush1298 deleted the add_revision_to_download_results branch December 18, 2025 15:08
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.

Allow users to specify revision for results repository

3 participants