Skip to content

Git Provider: do not pull after init. if pull fails, library is not ready.#753

Merged
eliska-n merged 1 commit intomasterfrom
fix/pull-after-init
May 7, 2026
Merged

Git Provider: do not pull after init. if pull fails, library is not ready.#753
eliska-n merged 1 commit intomasterfrom
fix/pull-after-init

Conversation

@eliska-n
Copy link
Copy Markdown
Collaborator

@eliska-n eliska-n commented May 4, 2026

I went through the git provider with several robots and this is the only bug I found and it is not in the code that would be used.
I failed to improve it more.
So this is it.

So, I am closing this MR: github.com//pull/751
It is redundant refactoring.

And I'll finish this: #751
as I didn't find any other line to change.

Let's see...

Summary by CodeRabbit

  • Bug Fixes
    • Optimized git repository initialization to defer pull operations to periodic syncs, improving startup efficiency by avoiding redundant network operations during initialization.

…gh the git is initialized. unrecoverable failure.
@eliska-n eliska-n requested review from ateska and mejroslav May 4, 2026 13:19
@eliska-n eliska-n self-assigned this May 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

The pull request modifies Git repository initialization to defer pulling until the periodic pull task runs. The initialize_git_repository() function now only clones missing or opens existing repositories without pulling, with updated documentation and atomic transition comments.

Changes

Git Provider Initialization Behavior

Layer / File(s) Summary
Behavior Change
asab/library/providers/git.py (lines 361–375)
The existing repository initialization path no longer calls _do_pull() immediately; cloning and opening remain but pulling is deferred.
Documentation & Comments
asab/library/providers/git.py (lines 356–359, 454–455)
Docstring and in-code comments updated to clarify that pulling is not performed during initialization, readiness is set once the repository is accessible, and that _set_ready() must be atomic with respect to the init task.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • mejroslav
  • ateska

Poem

🐰 A pull deferred is a pull refined,
No rushing clones with tangled vine,
Let init rest, let timers tick,
Periodic tasks do the trick!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main changes: stopping pull operations after init and ensuring the library only becomes ready when pull succeeds.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pull-after-init

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@asab/library/providers/git.py`:
- Around line 373-375: The provider can open a cached repo
(pygit2.Repository(self.RepoPath)) but may lack an "origin" remote causing a
KeyError later in _do_fetch when accessing self.GitRepository.remotes["origin"];
before calling _set_ready() validate that self.GitRepository.remotes contains
"origin" (e.g., check "origin" in self.GitRepository.remotes) and raise/handle a
descriptive error if missing so init fails fast, and update the exception
handling in _periodic_pull() to also catch KeyError (in addition to
pygit2.GitError) so a missing/corrupt origin is handled gracefully during
periodic fetchs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 089aa087-bea3-4d76-b9cf-2ae3247ad9e5

📥 Commits

Reviewing files that changed from the base of the PR and between bd01f5a and 74fcc5a.

📒 Files selected for processing (1)
  • asab/library/providers/git.py

Comment thread asab/library/providers/git.py
@eliska-n eliska-n merged commit 5cfcd60 into master May 7, 2026
14 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.

2 participants