Skip to content

refactor: dedupe store_*_bulk executemany blocks into BaseRepository.execute_bulk#1548

Closed
ebios-star wants to merge 1 commit into
entrius:testfrom
ebios-star:refactor/dedupe-bulk-upsert-executemany
Closed

refactor: dedupe store_*_bulk executemany blocks into BaseRepository.execute_bulk#1548
ebios-star wants to merge 1 commit into
entrius:testfrom
ebios-star:refactor/dedupe-bulk-upsert-executemany

Conversation

@ebios-star

Copy link
Copy Markdown
Contributor

Summary

The three Repository.store_*_bulk methods each repeated the same try / executemany / commit / rollback / return-count block verbatim:

  • gittensor/validator/storage/repository.pystore_pull_requests_bulk, store_issues_bulk, store_file_changes_bulk

The only things that varied per site were the query constant, the entity name in the error message, and — for file changes only — one extra | PRs: {...} log detail. This PR centralizes the block as BaseRepository.execute_bulk(), the executemany sibling of the existing execute_command() helper that already lives on the same base class. Each call site collapses from an 11-line block to a single delegating call.

def execute_bulk(
    self,
    query: str,
    values: List[tuple],
    entity_label: str,
    commit: bool = True,
    error_detail_fn: Optional[Callable[[], str]] = None,
) -> int:
    try:
        with self.get_cursor() as cursor:
            cursor.executemany(query, values)
            if commit:
                self.db.commit()
            return len(values)
    except Exception as e:
        if commit:
            self.db.rollback()
        detail = error_detail_fn() if error_detail_fn else ''
        self.logger.error(f'Error in bulk {entity_label} storage: {e}{detail}')
        return 0

Behavior is unchanged. Return values (len(values) on success, 0 on failure), commit/rollback semantics, and the exact error-log strings are all preserved. store_file_changes_bulk kept its | PRs: {...} context via the optional error_detail_fn, which — like the original — is only evaluated on the failure path, so the success path does no extra work.

The one structurally-different sibling is intentionally left untouched, since folding it in would change behavior:

  • set_miner_evaluation runs the same executemany shape but returns bool (True/False), not a row count, and builds its rows from master_repositories. Its contract differs, so it stays as-is.

Related Issues

None.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Testing

  • Tests added/updated
  • Manually tested

Added tests/validator/test_execute_bulk.py covering the new helper directly: success returns the row count and commits, commit=False skips the commit, the failure path rolls back and returns 0, and error_detail_fn is invoked only on failure. The existing store_*_bulk call sites are mocked at the boundary in the current suite, so the helper's internals were previously uncovered.

Full suite passes (uv run pytest → 956 passed). ruff check, ruff format --check, and pyright are all clean. This is an internal validator/storage refactor with no CLI/output changes, so the CLI before/after evidence requirement does not apply.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

…execute_bulk

The three Repository.store_*_bulk methods repeated the same
try/executemany/commit/rollback/return-count block verbatim, differing only
in the query, the entity name in the error log, and (for file changes) one
extra log detail. Centralize the pattern as BaseRepository.execute_bulk,
the executemany sibling of the existing execute_command helper.

Behavior is unchanged. store_file_changes_bulk keeps its lazy " | PRs: {...}"
error context via the optional error_detail_fn, evaluated only on failure.
set_miner_evaluation is intentionally left untouched: it returns bool, not a
row count, so folding it in would change its contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ebios-star

Copy link
Copy Markdown
Contributor Author

@anderdc @LandynDev whenever you have a moment — this is a small, behavior-preserving consolidation of the three store_*_bulk methods into a single BaseRepository.execute_bulk helper (the executemany sibling of the existing execute_command). Error strings, return values, and commit/rollback semantics are unchanged; set_miner_evaluation is intentionally left out since it returns a bool rather than a row count. Full suite green (956 passed). Happy to adjust scope or naming. Thanks!

@xiao-xiao-mao xiao-xiao-mao Bot added the refactor Code restructuring without behavior change label Jun 26, 2026
@anderdc anderdc closed this Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code restructuring without behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants