Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recovery functionality for Git #838

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Conversation

jjmerchante
Copy link
Contributor

This PR implements the recovery functionality in the fetch method of the Git backend.

It also includes the last item search fields in the summary to allow recovering from the last item.

Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

It mostly looks good. I added some comment to improve its readability.

perceval/backends/core/git.py Outdated Show resolved Hide resolved
perceval/backends/core/git.py Outdated Show resolved Hide resolved
perceval/backends/core/git.py Outdated Show resolved Hide resolved
perceval/backends/core/git.py Outdated Show resolved Hide resolved
Comment on lines 339 to 341
hashes = repo.get_commits_from_packs(packs, from_commit)
gitshow = repo.show(hashes)
commits = self.parse_git_log_from_iter(gitshow)
Copy link
Member

Choose a reason for hiding this comment

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

We might want to encapsulate these under a new method __fetch_from_packs(...).

@@ -706,6 +706,95 @@ def test_fetch_from_file(self):
self.assertEqual(commit['category'], 'commit')
self.assertEqual(commit['tag'], 'http://example.com.git')

def test_fetch_recovery(self):
Copy link
Member

Choose a reason for hiding this comment

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

The test looks great but I think you can improve it splitting in on different cases and not having everything tested on the same one.

@@ -1035,6 +1035,7 @@ def __init__(self):
self.max_offset = None
self.last_offset = None
self.extras = None
self.last_search_fields = None
Copy link
Member

Choose a reason for hiding this comment

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

Why is this field needed?

Copy link
Contributor Author

@jjmerchante jjmerchante Mar 12, 2024

Choose a reason for hiding this comment

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

The summary does not contain any information related to the last commit or the last item collected. I think this field (search_fields) contains that information for all the backends and can be used later in the scheduler to identify it.

@coveralls
Copy link

Coverage Status

coverage: 92.396% (-0.1%) from 92.521%
when pulling c52b82a on jjmerchante:recovery-git
into 1a3b337 on chaoss:master.

@coveralls
Copy link

coveralls commented Mar 11, 2024

Coverage Status

coverage: 92.385% (-0.1%) from 92.521%
when pulling 2db6922 on jjmerchante:recovery-git
into 319ae97 on chaoss:master.

This commit implements the recovery functionality in the fetch method
of the Git backend, enabling the recovery of Perceval execution from
a specific commit. Now, users can specify a recovery commit to start
fetching commits from that point, ensuring robustness against failures.

Signed-off-by: Jose Javier Merchante <jjmerchante@bitergia.com>
Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

LGTM

@sduenas sduenas merged commit 5ab5dcf into chaoss:master Mar 12, 2024
6 of 7 checks passed
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.

3 participants