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.

Sign up for GitHub

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

Jump to bottom

Warn on & skip workflow runs for certain "broken" GitHub workflows #151

Merged
merged 2 commits into from
Oct 11, 2022
Merged

Warn on & skip workflow runs for certain "broken" GitHub workflows #151

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d3ef6e
Warn on & skip workflow runs for certain "broken" workflows
jwodder Oct 10, 2022
74066c7
Also skip on other 5xx errors
jwodder Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 42 additions & 7 deletions src/tinuous/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from github import Github
from github.GitRelease import GitRelease
from github.GitReleaseAsset import GitReleaseAsset
from github.GithubException import RateLimitExceededException
from github.GithubException import RateLimitExceededException, UnknownObjectException
from github.Repository import Repository
from github.Workflow import Workflow
from github.WorkflowRun import WorkflowRun
from pydantic import BaseModel, Field
import requests
from urllib3.exceptions import ResponseError
from urllib3.util.retry import Retry

from .base import (
Expand All @@ -34,6 +35,12 @@
sanitize_pathname,
)

RETRY_STATUSES = [500, 502, 503, 504]
Copy link
Member

Choose a reason for hiding this comment

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

eh, I was trying to avoid "equating" the RETRY_STATUSES to all the statues we might want to skip on (only 5xxs). ATM they are indeed identically but in general not necessarily the same, so IMHO it mixes the semantics abit ... but since functionally is ok ATM, let's proceed. Thank you @jwodder ! Merging


RETRY_RESPONSE_ERRORS = {
ResponseError.SPECIFIC_ERROR.format(status_code=c): c for c in RETRY_STATUSES
}


def retry_ratelimit(func: Callable) -> Callable:
@wraps(func)
Expand Down Expand Up @@ -66,7 +73,7 @@ def client(self) -> Github:
retry=Retry(
total=12,
backoff_factor=1.25,
status_forcelist=[500, 502, 503, 504],
status_forcelist=RETRY_STATUSES,
),
)

Expand Down Expand Up @@ -98,11 +105,39 @@ def get_workflows(self) -> List[Workflow]:
@retry_ratelimit
def get_runs(self, wf: Workflow, since: datetime) -> List[WorkflowRun]:
runs: List[WorkflowRun] = []
for r in wf.get_runs():
if ensure_aware(r.created_at) <= since:
break
runs.append(r)
return runs
try:
for r in wf.get_runs():
if ensure_aware(r.created_at) <= since:
break
runs.append(r)
return runs
except requests.exceptions.RetryError as e:
reason = e.args[0].reason
if (
isinstance(reason, ResponseError)
and reason.args[0] in RETRY_RESPONSE_ERRORS
and ensure_aware(wf.updated_at) <= since
and not (wf.path and self.has_file(wf.path))
):
log.warning(
"Request for runs for workflow %s (%s) returned %d, and"
" workflow was deleted before starting timestamp; skipping",
wf.path,
wf.name,
RETRY_RESPONSE_ERRORS[reason.args[0]],
)
return []
else:
raise

@retry_ratelimit
def has_file(self, path: str) -> bool:
try:
self.ghrepo.get_contents(path)
except UnknownObjectException:
return False
else:
return True

def get_build_assets(
self, event_types: List[EventType], logs: bool, artifacts: bool
Expand Down