Add timing keyword argument for hooks

Measure the duration of the sync operation in the Execute method of the
Sync command and pass it to post-sync hooks as a standard keyword
argument (`sync_duration_seconds`).

Updates based on code review:
- Update _API_ARGS in hooks.py to allow sync_duration_seconds for post-sync hooks.
- Do not cast sync_duration_seconds to int for better granularity.
- Update docs/repo-hooks.md to document sync_duration_seconds.
- Add unit test for argument validation in test_hooks.py.

Test: Ran run_tests using venv python, all 554 tests passed.
Bug: TBD
Change-Id: Ie29e002a5d283460d993ad96c224dbf4b6d7985c
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/575021
Tested-by: Arif Kasim <arifkasim@google.com>
Commit-Queue: Ram Peri <ramperi@google.com>
Reviewed-by: Arif Kasim <arifkasim@google.com>
Reviewed-by: Gavin Mak <gavinmak@google.com>
This commit is contained in:
Ram Peri
2026-04-21 00:02:47 +00:00
committed by gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com
parent baa281d99e
commit 5af71ce907
4 changed files with 58 additions and 5 deletions
+2 -1
View File
@@ -163,13 +163,14 @@ Example:
The `post-sync.py` file should be defined like: The `post-sync.py` file should be defined like:
```py ```py
def main(repo_topdir=None, **kwargs): def main(repo_topdir=None, sync_duration_seconds=None, **kwargs):
"""Main function invoked directly by repo. """Main function invoked directly by repo.
We must use the name "main" as that is what repo requires. We must use the name "main" as that is what repo requires.
Args: Args:
repo_topdir: The absolute path to the top-level directory of the repo workspace. repo_topdir: The absolute path to the top-level directory of the repo workspace.
sync_duration_seconds: The duration of the sync operation in seconds.
kwargs: Leave this here for forward-compatibility. kwargs: Leave this here for forward-compatibility.
""" """
``` ```
+1 -1
View File
@@ -25,7 +25,7 @@ from git_refs import HEAD
# The API we've documented to hook authors. Keep in sync with repo-hooks.md. # The API we've documented to hook authors. Keep in sync with repo-hooks.md.
_API_ARGS = { _API_ARGS = {
"pre-upload": {"project_list", "worktree_list"}, "pre-upload": {"project_list", "worktree_list"},
"post-sync": {"repo_topdir"}, "post-sync": {"repo_topdir", "sync_duration_seconds"},
} }
+8 -3
View File
@@ -1986,17 +1986,19 @@ later is required to fix a server side protocol bug.
def Execute(self, opt, args): def Execute(self, opt, args):
errors = [] errors = []
start_time = time.time()
try: try:
self._ExecuteHelper(opt, args, errors) self._ExecuteHelper(opt, args, errors)
sync_duration_seconds = time.time() - start_time
except (RepoExitError, RepoChangedException): except (RepoExitError, RepoChangedException):
raise raise
except (KeyboardInterrupt, Exception) as e: except (KeyboardInterrupt, Exception) as e:
raise RepoUnhandledExceptionError(e, aggregate_errors=errors) raise RepoUnhandledExceptionError(e, aggregate_errors=errors)
# Run post-sync hook only after successful sync # Run post-sync hook only after successful sync
self._RunPostSyncHook(opt) self._RunPostSyncHook(opt, sync_duration_seconds=sync_duration_seconds)
def _RunPostSyncHook(self, opt): def _RunPostSyncHook(self, opt, sync_duration_seconds=None):
"""Run post-sync hook if configured in manifest <repo-hooks>.""" """Run post-sync hook if configured in manifest <repo-hooks>."""
hook = RepoHook.FromSubcmd( hook = RepoHook.FromSubcmd(
hook_type="post-sync", hook_type="post-sync",
@@ -2004,7 +2006,10 @@ later is required to fix a server side protocol bug.
opt=opt, opt=opt,
abort_if_user_denies=False, abort_if_user_denies=False,
) )
success = hook.Run(repo_topdir=self.client.topdir) success = hook.Run(
repo_topdir=self.client.topdir,
sync_duration_seconds=sync_duration_seconds,
)
if not success: if not success:
print("Warning: post-sync hook reported failure.") print("Warning: post-sync hook reported failure.")
+47
View File
@@ -14,6 +14,9 @@
"""Unittests for the hooks.py module.""" """Unittests for the hooks.py module."""
from io import StringIO
import sys
import pytest import pytest
import hooks import hooks
@@ -58,3 +61,47 @@ def test_direct_interp(shebang: str, interp: str) -> None:
def test_env_interp(shebang: str, interp: str) -> None: def test_env_interp(shebang: str, interp: str) -> None:
"""Lines whose shebang launches through `env`.""" """Lines whose shebang launches through `env`."""
assert hooks.RepoHook._ExtractInterpFromShebang(shebang) == interp assert hooks.RepoHook._ExtractInterpFromShebang(shebang) == interp
def test_post_sync_argument_validation() -> None:
"""Test that post-sync hook requires exact API arguments."""
class FakeProject:
def __init__(self):
self.worktree = "/some/path"
self.enabled_repo_hooks = ["post-sync"]
hook = hooks.RepoHook(
hook_type="post-sync",
hooks_project=FakeProject(),
repo_topdir="/topdir",
manifest_url="https://gerrit",
allow_all_hooks=True,
)
old_stderr = sys.stderr
sys.stderr = StringIO()
try:
# Call with missing arg `sync_duration_seconds`
res = hook.Run(repo_topdir="/topdir")
assert res is False
assert "hook 'post-sync' called incorrectly" in sys.stderr.getvalue()
# Mock _CheckHook and _ExecuteHook to test success path
hook._CheckHook = lambda: None
executed_kwargs = {}
def fake_execute(**kw):
executed_kwargs.update(kw)
hook._ExecuteHook = fake_execute
res = hook.Run(repo_topdir="/topdir", sync_duration_seconds=12.345)
assert res is True
assert executed_kwargs.get("sync_duration_seconds") == 12.345
finally:
sys.stderr = old_stderr