-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Auto Build Documentation on Pull Request (PR Builder) #5828
Changes from all commits
be8f4de
30ac6ba
df122da
da4f71d
1207afb
c74d294
5fb2e4e
a859eac
5b883df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,11 @@ | |
webhook_github, | ||
webhook_gitlab, | ||
) | ||
from readthedocs.core.views.hooks import build_branches, sync_versions | ||
from readthedocs.core.views.hooks import ( | ||
build_branches, | ||
sync_versions, | ||
get_or_create_external_version, | ||
) | ||
from readthedocs.integrations.models import HttpExchange, Integration | ||
from readthedocs.projects.models import Project | ||
|
||
|
@@ -29,6 +33,9 @@ | |
GITHUB_EVENT_HEADER = 'HTTP_X_GITHUB_EVENT' | ||
GITHUB_SIGNATURE_HEADER = 'HTTP_X_HUB_SIGNATURE' | ||
GITHUB_PUSH = 'push' | ||
GITHUB_PULL_REQUEST = 'pull_request' | ||
GITHUB_PULL_REQUEST_OPEN = 'opened' | ||
GITHUB_PULL_REQUEST_SYNC = 'synchronize' | ||
GITHUB_CREATE = 'create' | ||
GITHUB_DELETE = 'delete' | ||
GITLAB_TOKEN_HEADER = 'HTTP_X_GITLAB_TOKEN' | ||
|
@@ -110,6 +117,10 @@ def handle_webhook(self): | |
"""Handle webhook payload.""" | ||
raise NotImplementedError | ||
|
||
def get_external_version_data(self): | ||
"""Get External Version data from payload.""" | ||
raise NotImplementedError | ||
|
||
def is_payload_valid(self): | ||
"""Validates the webhook's payload using the integration's secret.""" | ||
return False | ||
|
@@ -218,6 +229,13 @@ def get_data(self): | |
pass | ||
return super().get_data() | ||
|
||
def get_external_version_data(self): | ||
"""Get Commit Sha and pull request number from payload""" | ||
identifier = self.data['pull_request']['head']['sha'] | ||
verbose_name = str(self.data['number']) | ||
|
||
return identifier, verbose_name | ||
|
||
def is_payload_valid(self): | ||
""" | ||
GitHub use a HMAC hexdigest hash to sign the payload. | ||
|
@@ -271,6 +289,21 @@ def handle_webhook(self): | |
raise ParseError('Parameter "ref" is required') | ||
if event in (GITHUB_CREATE, GITHUB_DELETE): | ||
return self.sync_versions(self.project) | ||
|
||
if ( | ||
event == GITHUB_PULL_REQUEST and | ||
self.data['action'] in [GITHUB_PULL_REQUEST_OPEN, GITHUB_PULL_REQUEST_SYNC] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will handle both PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tested it locally and it worked great 👍 |
||
): | ||
try: | ||
identifier, verbose_name = self.get_external_version_data() | ||
external_version = get_or_create_external_version( | ||
self.project, identifier, verbose_name | ||
) | ||
return self.get_response_push(self.project, [external_version.verbose_name]) | ||
|
||
except KeyError: | ||
raise ParseError('Parameters "sha" and "number" are required') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Error message can be changed |
||
|
||
return None | ||
|
||
def _normalize_ref(self, ref): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
|
||
import logging | ||
|
||
from readthedocs.builds.constants import EXTERNAL | ||
from readthedocs.builds.models import Version | ||
from readthedocs.core.utils import trigger_build | ||
from readthedocs.projects.tasks import sync_repository_task | ||
|
||
|
@@ -88,3 +90,21 @@ def sync_versions(project): | |
except Exception: | ||
log.exception('Unknown sync versions exception') | ||
return None | ||
|
||
|
||
def get_or_create_external_version(project, identifier, verbose_name): | ||
external_version = project.versions(manager=EXTERNAL).filter(verbose_name=verbose_name).first() | ||
if external_version: | ||
if external_version.identifier != identifier: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be updated when there is a new commit in the PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably better in a code comment, not a PR comment that will be lost :) |
||
external_version.identifier = identifier | ||
external_version.save() | ||
else: | ||
created_external_version = Version.objects.create( | ||
project=project, | ||
type=EXTERNAL, | ||
identifier=identifier, | ||
verbose_name=verbose_name, | ||
active=True | ||
) | ||
return created_external_version | ||
return external_version |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,7 +138,7 @@ def sync_repo(self): | |
} | ||
) | ||
version_repo = self.get_vcs_repo() | ||
version_repo.update() | ||
version_repo.update(version=self.version) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to pass the version here so that we can determine which PR should be fetched. there previous implementation fetched all the PR's all together regardless it was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a good and small change 👍 |
||
self.sync_versions(version_repo) | ||
version_repo.checkout(self.version.identifier) | ||
finally: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I'm considering
PR_Number
as verbose name.