Skip to content

Try unpinning testing reqs #2863

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 34 additions & 1 deletion prospector-more.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,39 @@
inherits: prospector

strictness: medium
strictness: high

ignore-paths:
- constants.py
- urls.py
- wsgi.py
- acl
- api
- betterversion
- bookmarks
- builds
- cdn
- comments
- core
- djangome
- doc_builder
- donate
- gold
- integrations
- locale
- notifications
- oauth
- payments
- privacy
- profiles
- projects
- redirects
- restapi
- rtd_tests
- search
- settings
- tastyapi
- templates
- vcs_support
Copy link
Member

Choose a reason for hiding this comment

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

Rinse and repeat :)


pylint:
options:
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def is_authenticated(self, request, **kwargs):


class EnhancedModelResource(ModelResource):
def obj_get_list(self, request=None, *_, **kwargs):
def obj_get_list(self, request=None, *_, **kwargs): # pylint: disable=arguments-differ
"""
A ORM-specific implementation of ``obj_get_list``.

Expand Down
65 changes: 32 additions & 33 deletions readthedocs/bookmarks/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
class BookmarkExistsView(View):

@method_decorator(csrf_exempt)
def dispatch(self, *args, **kwargs):
return super(BookmarkExistsView, self).dispatch(*args, **kwargs)
def dispatch(self, request, *args, **kwargs):
return super(BookmarkExistsView, self).dispatch(request, *args, **kwargs)

def get(self, request):
return HttpResponse(
Expand Down Expand Up @@ -80,8 +80,8 @@ class BookmarkListView(ListView):
model = Bookmark

@method_decorator(login_required)
def dispatch(self, *args, **kwargs):
return super(BookmarkListView, self).dispatch(*args, **kwargs)
def dispatch(self, request, *args, **kwargs):
return super(BookmarkListView, self).dispatch(request, *args, **kwargs)

def get_queryset(self):
return Bookmark.objects.filter(user=self.request.user)
Expand All @@ -93,8 +93,8 @@ class BookmarkAddView(View):

@method_decorator(login_required)
@method_decorator(csrf_exempt)
def dispatch(self, *args, **kwargs):
return super(BookmarkAddView, self).dispatch(*args, **kwargs)
def dispatch(self, request, *args, **kwargs):
return super(BookmarkAddView, self).dispatch(request, *args, **kwargs)

def get(self, request):
return HttpResponse(
Expand Down Expand Up @@ -156,8 +156,8 @@ class BookmarkRemoveView(View):

@method_decorator(login_required)
@method_decorator(csrf_exempt)
def dispatch(self, *args, **kwargs):
return super(BookmarkRemoveView, self).dispatch(*args, **kwargs)
def dispatch(self, request, *args, **kwargs):
return super(BookmarkRemoveView, self).dispatch(request, *args, **kwargs)

def get(self, request, *args, **kwargs):
return render_to_response(
Expand All @@ -175,30 +175,29 @@ def post(self, request, *args, **kwargs):
bookmark = get_object_or_404(Bookmark, pk=kwargs['bookmark_pk'])
bookmark.delete()
return HttpResponseRedirect(reverse('bookmark_list'))
else:
try:
post_json = json.loads(request.body)
project = Project.objects.get(slug=post_json['project'])
version = project.versions.get(slug=post_json['version'])
url = post_json['url']
page = post_json['page']
except KeyError:
return HttpResponseBadRequest(
json.dumps({'error': "Invalid parameters"})
)

bookmark = get_object_or_404(
Bookmark,
user=request.user,
url=url,
project=project,
version=version,
page=page
try:
post_json = json.loads(request.body)
project = Project.objects.get(slug=post_json['project'])
version = project.versions.get(slug=post_json['version'])
url = post_json['url']
page = post_json['page']
except KeyError:
return HttpResponseBadRequest(
json.dumps({'error': "Invalid parameters"})
)
bookmark.delete()

return HttpResponse(
json.dumps({'removed': True}),
status=200,
content_type="application/json"
)
bookmark = get_object_or_404(
Bookmark,
user=request.user,
url=url,
project=project,
version=version,
page=page
)
bookmark.delete()

return HttpResponse(
json.dumps({'removed': True}),
status=200,
content_type="application/json"
)
Copy link
Member

Choose a reason for hiding this comment

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

Should we just delete this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps with another PR? I don't think the bookmarks code needs to live in our repo

4 changes: 2 additions & 2 deletions readthedocs/builds/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class Meta:
model = Version
fields = ['active', 'privacy_level', 'tags']

def save(self, *args, **kwargs):
obj = super(VersionForm, self).save(*args, **kwargs)
def save(self, commit=True):
obj = super(VersionForm, self).save(commit=commit)
if obj.active and not obj.built and not obj.uploaded:
trigger_build(project=obj.project, version=obj)
return obj
Copy link
Member

Choose a reason for hiding this comment

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

I do worry that this class of change will have spiraling effects that only show up in run time. Is there a reason we don't want to continue accepting args & kwargs for these things?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, we should have never pushed *args, **kwargs, as it was just lazy. commit=True is more explicit and is now identical to the underlying method signature.

In cases where there are many arguments, I don't know what's best there. *args, **kwargs is flexible for future compatibility, so probably in that case it's fine to ignore the linting error and use the *args, **kwargs catch all.

7 changes: 3 additions & 4 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ def commit_name(self):
if self.slug == LATEST:
if self.project.default_branch:
return self.project.default_branch
else:
return self.project.vcs_repo().fallback_branch
return self.project.vcs_repo().fallback_branch

if self.slug == STABLE:
if self.type == BRANCH:
Expand Down Expand Up @@ -144,7 +143,7 @@ def get_absolute_url(self):
private = self.privacy_level == PRIVATE
return self.project.get_docs_url(version_slug=self.slug, private=private)

def save(self, *args, **kwargs):
def save(self, *args, **kwargs): # pylint: disable=arguments-differ
"""Add permissions to the Version for all owners on save."""
from readthedocs.projects import tasks
obj = super(Version, self).save(*args, **kwargs)
Expand All @@ -157,7 +156,7 @@ def save(self, *args, **kwargs):
broadcast(type='app', task=tasks.symlink_project, args=[self.project.pk])
return obj

def delete(self, *args, **kwargs):
def delete(self, *args, **kwargs): # pylint: disable=arguments-differ
from readthedocs.projects import tasks
log.info('Removing files for version %s', self.slug)
tasks.clear_artifacts.delay(version_pk=self.pk)
Expand Down
27 changes: 12 additions & 15 deletions readthedocs/comments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ def from_hash(self, version_slug, page, node_hash, project_slug=None):
if project_slug:
snapshots = snapshots.filter(node__project__slug=project_slug)

if len(snapshots) == 0:
if not snapshots.exists():
raise DocumentNode.DoesNotExist(
"No node exists on %s with a current hash of %s" % (
page, node_hash))

if len(snapshots) == 1:
if snapshots.count() == 1:
# If we have found only one snapshot, we know we have the correct node.
node = snapshots[0].node
else:
Expand Down Expand Up @@ -84,22 +84,20 @@ def latest_commit(self):
def visible_comments(self):
if not self.project.comment_moderation:
return self.comments.all()
else:
# non-optimal SQL warning.
decisions = ModerationAction.objects.filter(
comment__node=self,
decision=1,
date__gt=self.snapshots.latest().date
)
valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct()
return valid_comments
# non-optimal SQL warning.
decisions = ModerationAction.objects.filter(
comment__node=self,
decision=1,
date__gt=self.snapshots.latest().date
)
valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct()
return valid_comments

def update_hash(self, new_hash, commit):
latest_snapshot = self.snapshots.latest()
if latest_snapshot.hash == new_hash and latest_snapshot.commit == commit:
return latest_snapshot
else:
return self.snapshots.create(hash=new_hash, commit=commit)
return self.snapshots.create(hash=new_hash, commit=commit)


class DocumentNodeSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -183,8 +181,7 @@ def has_been_approved_since_most_recent_node_change(self):
# If we do have an approval action which is newer than the most recent change,
# we'll return True or False commensurate with its "approved" attribute.
return latest_moderation_action.approved()
else:
return False
return False

def is_orphaned(self):
raise NotImplementedError('TODO')
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/comments/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def get_queryset(self):
return queryset

@method_decorator(login_required)
def create(self, request):
def create(self, request, *args, **kwargs):
project = Project.objects.get(slug=request.data['project'])
comment = project.add_comment(version_slug=request.data['version'],
page=request.data['document_page'],
Expand Down
6 changes: 3 additions & 3 deletions readthedocs/core/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ def __init__(self, *args, **kwargs):
except AttributeError:
pass

def save(self, *args, **kwargs):
def save(self, commit=True):
first_name = self.cleaned_data.pop('first_name', None)
last_name = self.cleaned_data.pop('last_name', None)
profile = super(UserProfileForm, self).save(*args, **kwargs)
if kwargs.get('commit', True):
profile = super(UserProfileForm, self).save(commit=commit)
if commit:
user = profile.user
user.first_name = first_name
user.last_name = last_name
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/management/commands/import_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Command(BaseCommand):
help = __doc__

def handle(self, *args, **options):
if len(args):
if args:
for slug in args:
for service in GitHubService.for_user(
User.objects.get(username=slug)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/management/commands/pull.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Command(BaseCommand):
help = __doc__

def handle(self, *args, **options):
if len(args):
if args:
for slug in args:
tasks.update_imported_docs(
utils.version_from_slug(slug, LATEST).pk
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/management/commands/update_repos.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def handle(self, *args, **options):
record = options['record']
force = options['force']
version = options['version']
if len(args):
if args:
for slug in args:
if version and version != "all":
log.info("Updating version %s for %s", version, slug)
Expand Down
11 changes: 3 additions & 8 deletions readthedocs/core/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ def resolve_path(self, project, filename='', version_slug=None,
project_slug = project.slug
subproject_slug = None

if project.single_version or single_version:
single_version = True
else:
single_version = False
single_version = bool(project.single_version or single_version)

return self.base_resolve_path(
project_slug=project_slug,
Expand All @@ -128,8 +125,7 @@ def resolve_domain(self, project, private=None):
return domain.domain
elif self._use_subdomain():
return self._get_project_subdomain(canonical_project)
else:
return getattr(settings, 'PRODUCTION_DOMAIN')
return getattr(settings, 'PRODUCTION_DOMAIN')

def resolve(self, project, protocol='http', filename='', private=None,
**kwargs):
Expand Down Expand Up @@ -158,8 +154,7 @@ def _get_canonical_project(self, project):
return main_language_project
elif relation:
return relation.parent
else:
return project
return project

def _get_project_subdomain(self, project):
"""Determine canonical project domain as subdomain"""
Expand Down
8 changes: 4 additions & 4 deletions readthedocs/core/symlink.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,15 @@ def symlink_cnames(self, domain=None):
domains = [domain]
else:
domains = Domain.objects.filter(project=self.project)
for domain in domains:
self._log(u"Symlinking CNAME: {0} -> {1}".format(domain.domain, self.project.slug))
for dom in domains:
self._log(u"Symlinking CNAME: {0} -> {1}".format(dom.domain, self.project.slug))

# CNAME to doc root
symlink = os.path.join(self.CNAME_ROOT, domain.domain)
symlink = os.path.join(self.CNAME_ROOT, dom.domain)
run('ln -nsf {0} {1}'.format(self.project_root, symlink))

# Project symlink
project_cname_symlink = os.path.join(self.PROJECT_CNAME_ROOT, domain.domain)
project_cname_symlink = os.path.join(self.PROJECT_CNAME_ROOT, dom.domain)
run('ln -nsf %s %s' % (self.project.doc_path, project_cname_symlink))

def remove_symlink_cname(self, domain):
Expand Down
8 changes: 3 additions & 5 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ def run_on_app_servers(command):
if ret != 0:
ret_val = ret
return ret_val
else:
ret = os.system(command)
return ret
ret = os.system(command)
return ret


def broadcast(type, task, args): # pylint: disable=redefined-builtin
Expand All @@ -56,8 +55,7 @@ def clean_url(url):
parsed = urlparse(url)
if parsed.scheme or parsed.netloc:
return parsed.netloc
else:
return parsed.path
return parsed.path


def cname_to_slug(host):
Expand Down
5 changes: 2 additions & 3 deletions readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,8 @@ def wipe_version(request, project_slug, version_slug):
for del_dir in del_dirs:
broadcast(type='build', task=remove_dir, args=[del_dir])
return redirect('project_version_list', project_slug)
else:
return render_to_response('wipe_version.html',
context_instance=RequestContext(request))
return render_to_response('wipe_version.html',
context_instance=RequestContext(request))


def divide_by_zero(request): # pylint: disable=unused-argument
Expand Down
Loading