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.

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

Chain of Trust on Github: Migrate to the v4 API #367

Open
JohanLorenzo opened this issue Jul 17, 2019 · 0 comments
Open

Chain of Trust on Github: Migrate to the v4 API #367

JohanLorenzo opened this issue Jul 17, 2019 · 0 comments

Comments

@JohanLorenzo
Copy link
Contributor

JohanLorenzo commented Jul 17, 2019

We're using the v3 API, because that was the one officially supported at the time.

This led us (for instance) to use this non-API call:

async def has_commit_landed_on_repository(self, context, revision):
"""Tell if a commit was landed on the repository or if it just comes from a pull request.
Args:
context (scriptworker.context.Context): the scriptworker context.
revision (str): the commit hash or the tag name.
Returns:
bool: True if the commit is present in one of the branches of the main repository
"""
# Revision may be a tag name. `branch_commits` doesn't work on tags
if not _is_git_full_hash(revision):
revision = self.get_tag_hash(tag_name=revision)
repo = self._github_repository.html_url
url = '/'.join([repo.rstrip('/'), 'branch_commits', revision])
from scriptworker.task import get_decision_task_id
cache_key = '{}-{}'.format(get_decision_task_id(context.task), url)
async with _branch_commits_cache_lock:
if cache_key in _branch_commits_cache:
html_text = _branch_commits_cache[cache_key]
else:
html_data = await retry_request(context, url)
html_text = html_data.strip()
_branch_commits_cache[cache_key] = html_text
# https://github.com/{repo_owner}/{repo_name}/branch_commits/{revision} just returns some \n
# when the commit hasn't landed on the origin repo. Otherwise, some HTML data is returned - it
# represents the branches on which the given revision is present.
return html_text != ''

That call created issues like #331, where we ended up manually caching the responses.

Moving to the GraphQL API may help us in removing that caching logic

https://developer.github.com/v4/guides/migrating-from-rest/

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

No branches or pull requests

1 participant