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

Fire a signal for domain verification (eg. for SSL) #5071

Merged
merged 2 commits into from
Jan 15, 2019
Merged

Fire a signal for domain verification (eg. for SSL) #5071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df7548d
Fire a signal for domain verification (eg. for SSL)
davidfischer Jan 4, 2019
5bf7681
Use web queue on domain verification
ericholscher Jan 10, 2019
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
3 changes: 3 additions & 0 deletions readthedocs/projects/signals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@
project_import = django.dispatch.Signal(providing_args=["project"])

files_changed = django.dispatch.Signal(providing_args=["project", "files"])

# Used to force verify a domain (eg. for SSL cert issuance)
domain_verify = django.dispatch.Signal(providing_args=["domain"])
15 changes: 15 additions & 0 deletions readthedocs/projects/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
before_build,
before_vcs,
files_changed,
domain_verify,
)

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -1393,3 +1394,17 @@ def finish_inactive_builds():
'Builds marked as "Terminated due inactivity": %s',
builds_finished,
)


@app.task(queue='web')
def retry_domain_verification(domain_pk):
"""
Trigger domain verification on a domain

:param domain_pk: a `Domain` pk to verify
"""
domain = Domain.objects.get(pk=domain_pk)
domain_verify.send(
sender=domain.__class__,
domain=domain,
)
10 changes: 9 additions & 1 deletion readthedocs/projects/views/private.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
)
from readthedocs.projects.signals import project_import
from readthedocs.projects.views.base import ProjectAdminMixin, ProjectSpamMixin
from ..tasks import retry_domain_verification

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -726,7 +727,14 @@ def get_success_url(self):


class DomainList(DomainMixin, ListViewWithForm):
pass
def get_context_data(self, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to put this code under get method, don't you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that would be worse, actually. We need access to the domain_list from the context. The get method both calls get_context_data and consumes the context (but doesn't return it). As a result, we'd have to replace get and not merely call the super method and add functionality. Does that make sense?

ctx = super(DomainList, self).get_context_data(**kwargs)

# Retry validation on all domains if applicable
for domain in ctx['domain_list']:
retry_domain_verification.delay(domain_pk=domain.pk)

return ctx


class DomainCreate(DomainMixin, CreateView):
Expand Down