src/snakemake/sourcecache.py (3)

383-389: ⚠️ Potential issue | 🔴 Critical

partial(...) never executes the fetch; exception handling defeats retry logic.

The partial(self.repo.remotes.origin.fetch, ...) on line 386 only constructs a callable—it never invokes it (missing ()), so the fetch never runs yet _fetched = True is set unconditionally.

Additionally, catching GitCommandError and returning str(e) means tenacity sees the method succeed (returns a string) and will never retry on transient failures.

Proposed fix

         try:
-            # arg is needed in order to have a refspec, associated issue with
-            # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
-            partial(self.repo.remotes.origin.fetch, "+refs/heads/*:refs/heads/*")
+            # refspec arg needed per GitPython issue `#296`
+            self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
         except git.GitCommandError as e:
-            return str(e)
+            raise  # Let tenacity handle retry
         self._fetched = True

If callers rely on the Optional[str] return contract, handle the final exception one level up (e.g., in fetch_if_required) after retries are exhausted.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 383 - 389, The code constructs a
callable with partial(self.repo.remotes.origin.fetch,
"+refs/heads/*:refs/heads/*") but never invokes it and then unconditionally sets
self._fetched and returns on GitCommandError, which defeats retry logic; change
this so you actually call the fetch (invoke the partial or call
self.repo.remotes.origin.fetch with the refspec), do not catch
git.GitCommandError here so tenacity can retry, and only set self._fetched =
True after a successful fetch; preserve the Optional[str] contract by
propagating or re-raising the final exception to be handled in fetch_if_required
(or return an error string there) rather than returning str(e) from this method.

---

766-771: ⚠️ Potential issue | 🔴 Critical

Decorator references undefined retries—breaks module import.

@retry(..., stop=stop_after_attempt(retries), ...) is evaluated at class definition time, but retries doesn't exist in module scope—it was presumably intended to be a parameter. This causes the NameError shown in the pipeline failure. Additionally, logging.DEBUG requires importing logging.

If a fixed retry count is acceptable, hardcode it. If per-call configuration is needed, use tenacity's runtime API instead:

Option 1: Hardcoded retry count (simpler)

+import logging
 from tenacity import retry, stop_after_attempt, wait_exponential, after_log
...
 `@retry`(
     wait=wait_exponential(multiplier=2, min=3),
-    stop=stop_after_attempt(retries),
+    stop=stop_after_attempt(3),
     after=after_log(logger, logging.DEBUG),
 )
 def _open(self, source_file: SourceFile, mode, encoding=None):

Option 2: Runtime retry for per-call configuration

from tenacity import Retrying, stop_after_attempt, wait_exponential

def _open(self, source_file: SourceFile, mode, encoding=None, retries: int = 3):
    for attempt in Retrying(
        wait=wait_exponential(multiplier=2, min=3),
        stop=stop_after_attempt(retries),
    ):
        with attempt:
            return self._do_open(source_file, mode, encoding)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 766 - 771, The decorator on _open
references an undefined retries and also uses logging without importing it; fix
by either (A) hardcoding a retry count in the decorator (e.g., replace
stop=stop_after_attempt(retries) with stop=stop_after_attempt(3)) and add
"import logging" to the module, or (B) remove the decorator and implement
runtime retries using tenacity.Retrying/stop_after_attempt with a retries
parameter (e.g., implement a loop in _open that calls self._do_open inside a
Retrying context); locate the tenacity decorator on the _open definition and the
_open function itself to apply the change. Ensure logging.DEBUG is available by
importing logging.

---

14-14: ⚠️ Potential issue | 🔴 Critical

Missing logging module import.

The after_log callback at line 769 uses logging.DEBUG, but the logging module is never imported. Only logger from snakemake.logging is available.

Proposed fix

+import logging
 from tenacity import retry, stop_after_attempt, wait_exponential, after_log

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` at line 14, The code uses after_log with
logging.DEBUG but never imports the logging module; add an import for the
standard logging library at the top of the file so the symbol logging.DEBUG is
defined (e.g., add "import logging" alongside other imports in sourcecache.py),
ensuring the retry(..., after=after_log(logger, logging.DEBUG)) call works
correctly with the logging module available.

src/snakemake/sourcecache.py (1)

766-770: Consider reraise=True to preserve original exception type for callers.

When retries are exhausted, tenacity raises RetryError by default, wrapping the last exception. Callers like report/__init__.py catch generic Exception and wrap in WorkflowError, but the original exception context may be obscured. Adding reraise=True to the decorator will re-raise the original exception instead:

`@retry`(
    wait=...,
    stop=...,
    reraise=True,  # Re-raise original exception after retry exhaustion
)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 766 - 770, The retry decorator on
the function in sourcecache.py currently lacks reraise=True, so when retries
exhaust tenacity wraps the last exception in RetryError; update the `@retry`(...)
decorator (the one using wait_exponential, stop_after_attempt and after_log with
logger) to include reraise=True so the original exception type is re-raised to
callers (preserving exception context for callers like report/__init__.py).

src/snakemake/sourcecache.py (2)

372-389: ⚠️ Potential issue | 🔴 Critical

Call origin.fetch() here and let tenacity see failures.

Lines 383-388 currently build a partial without executing it, and return str(e) hides the failure from tenacity. As written, existing clones stay stale and this retry wrapper never retries a failed fetch.

Minimal fix

-        try:
-            # arg is needed in order to have a refspec, associated issue with
-            # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
-            partial(self.repo.remotes.origin.fetch, "+refs/heads/*:refs/heads/*")
-        except git.GitCommandError as e:
-            return str(e)
+        # arg is needed in order to have a refspec, associated issue with
+        # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
+        self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
         self._fetched = True

If fetch_if_required() still needs an Optional[str], convert the final exception one level up after the retry loop finishes.

#!/bin/bash
sed -n '372,389p' src/snakemake/sourcecache.py

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 372 - 389, The fetch method
currently constructs a functools.partial for self.repo.remotes.origin.fetch but
never calls it and catches GitCommandError returning its string, which prevents
tenacity from retrying; replace the partial usage by actually calling
self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*") so exceptions
propagate to tenacity, remove the return str(e) in the except block (or re-raise
the exception) and only set self._fetched = True after a successful fetch so
failed fetches are retried by the `@retry` wrapper on fetch.

---

14-14: ⚠️ Potential issue | 🔴 Critical

Import logging before using logging.DEBUG.

Line 769 now references logging.DEBUG, but this module never imports logging. That makes sourcecache.py fail at import time with NameError.

Minimal fix

+import logging
 from tenacity import retry, stop_after_attempt, wait_exponential, after_log

#!/bin/bash
sed -n '1,25p' src/snakemake/sourcecache.py
sed -n '766,770p' src/snakemake/sourcecache.py

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` at line 14, The module uses logging.DEBUG
(referenced in the retry decorator or related code) but never imports the
logging module; add "import logging" alongside the other imports at the top of
src/snakemake/sourcecache.py (near the existing "from tenacity import retry,
stop_after_attempt, wait_exponential, after_log") so that logging.DEBUG is
defined when used (e.g., in the retry/after_log usage).

src/snakemake/sourcecache.py (2)

13-13: ⚠️ Potential issue | 🔴 Critical

Fix the _open() retry decorator before import.

Line 768 uses logger.INFO, but logger is a logging.Logger and has no INFO attribute, which matches the current pipeline failure. Also, stop_after_attempt(1) disables retries, and without reraise=True exhausted attempts surface as RetryError instead of the WorkflowError raised below.

Suggested fix

+import logging
 from tenacity import retry, stop_after_attempt, wait_exponential, after_log
@@
     `@retry`(
         wait=wait_exponential(multiplier=2, min=3),
-        stop=stop_after_attempt(1),
-        after=after_log(logger, logger.INFO),
+        stop=stop_after_attempt(3),
+        after=after_log(logger, logging.INFO),
+        reraise=True,
     )

Also applies to: 765-769

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` at line 13, The retry decorator applied to
_open() is incorrect: replace the non-existent logger.INFO with the proper
logging level constant (e.g., logging.INFO) or import the logging module and use
logging.INFO when calling after_log; change stop_after_attempt(1) to allow
retries (e.g., stop_after_attempt(3) or remove the single-attempt stopper) and
add reraise=True to the retry(...) args so that exhausted retries surface the
original WorkflowError instead of a tenacity.RetryError; update the decorator
import usage (retry, stop_after_attempt, wait_exponential, after_log) on the
_open function accordingly.

---

371-387: ⚠️ Potential issue | 🟠 Major

Let tenacity see fetch failures.

return str(e) turns a failed origin.fetch() into a successful call, so this decorator never retries and transient remote errors still fall through on the first attempt. If you still need the Optional[str] contract, convert the final failure to a message in fetch_if_required() after the retry loop, not inside fetch().

Minimal fix

     `@retry`(wait=wait_exponential(multiplier=2, min=3), stop=stop_after_attempt(3))
     def fetch(self) -> Optional[str]:
-        import git
-
         if self._fetched:
             # up to date, nothing to do
             return
@@
-        try:
-            # arg is needed in order to have a refspec, associated issue with
-            # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
-            self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
-        except git.GitCommandError as e:
-            return str(e)
+        # arg is needed in order to have a refspec, associated issue with
+        # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
+        self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
         self._fetched = True

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 371 - 387, The fetch method
currently catches git.GitCommandError and returns str(e), which prevents the
`@retry` decorator on fetch from seeing failures; remove the return str(e) and
re-raise the exception (e.g. raise or raise from e) so tenacity can retry
origin.fetch; keep the Optional[str] surface by moving error-to-string
conversion into the caller fetch_if_required() after fetch finishes (catch
exceptions there and return the error message), and ensure references to
repo.remotes.origin.fetch and the `@retry-decorated` fetch() are updated
accordingly.

src/snakemake/sourcecache.py (1)

784-805: Chain the underlying exception in both _open() handlers.

These two wrappers drop the original traceback right where the new retry path was added. Use raise ... from e in both handlers so the final failure still points to the underlying Git or I/O error.

Suggested fix

         except git.GitCommandError as e:
             raise WorkflowError(
                 f"Failed to get local git source file {log_path}: {e}. "
                 "Is the local git clone up to date?"
-            )
+            ) from e
@@
         except Exception as e:
-            raise WorkflowError(
-                f"Failed to open source file {log_path}",
-                e,
-            )
+            raise WorkflowError(f"Failed to open source file {log_path}") from e

Based on learnings: use exception chaining (raise NewError(...) from original) to preserve the original exception context when re-raising or wrapping exceptions in Python files.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 784 - 805, The WorkflowError
wrappers in the _open() handlers drop original tracebacks; update both exception
raises (the one catching git.GitCommandError as e and the generic except
Exception as e that wraps open(...)) to chain the original exception using
"raise WorkflowError(... ) from e" so the underlying git or I/O exception
context is preserved when re-raising.

src/snakemake/sourcecache.py (1)

382-388: ⚠️ Potential issue | 🟠 Major

Exception swallowing prevents tenacity from retrying.

The try/except block catches git.GitCommandError and returns str(e) instead of raising. Tenacity only retries on raised exceptions, so transient fetch failures will never be retried—the method "succeeds" from tenacity's perspective by returning a string.

Either re-raise the exception to let tenacity retry, or move the error handling outside the decorated method.

Option 1: Let tenacity retry by removing inner exception handling

     `@retry`(wait=wait_exponential(multiplier=2, min=3), stop=stop_after_attempt(3))
-    def fetch(self) -> Optional[str]:
+    def fetch(self) -> None:
         import git

         if self._fetched:
             # up to date, nothing to do
             return

         logger.info(
             f"Fetching latest changes of {self.host}/{self.repo_name} to {self.repo_clone}"
         )
-        try:
-            # arg is needed in order to have a refspec, associated issue with
-            # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
-            self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
-        except git.GitCommandError as e:
-            return str(e)
+        # arg is needed in order to have a refspec, associated issue with
+        # workaround is here: https://github.com/gitpython-developers/GitPython/issues/296
+        self.repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
         self._fetched = True

If callers need the Optional[str] contract for error reporting, wrap the call site with error handling instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/sourcecache.py` around lines 382 - 388, The fetch block
currently swallows git.GitCommandError by returning str(e), preventing tenacity
retries; change the handler in the method that calls
self.repo.remotes.origin.fetch (the method containing that try/except) to not
return the exception string—either remove the try/except so the
git.GitCommandError is propagated to tenacity, or re-raise the caught exception
(raise) instead of returning it; if you must preserve an Optional[str] return
for callers, move the try/except to the call site and convert the exception to a
string there so the tenacity-decorated function only raises on failures.