Make linter significantly more strict

.travis.yml

@@ -56,7 +56,7 @@
     - ./configure
     - pip-sync dev_requirements.txt
     - ./travis/check_for_dead_code_with_vulture.sh
-    - prospector --profile prospector_profile.yaml --messages-only
+    - prospector --profile prospector_profile.yaml --messages-only -- $(git ls-files | grep '\.py$')
     - coverage run manage.py test --keepdb --noinput --settings=atmosphere.settings
     - coverage run --append manage.py behave --keepdb --tags ~@skip-if-${DISTRIBUTION} --settings=atmosphere.settings --format rerun --outfile rerun_failing.features
     - if [ -f "rerun_failing.features" ]; then python manage.py behave --logging-level DEBUG --capture-stderr --capture --verbosity 3 --keepdb @rerun_failing.features; fi

CHANGELOG.md

@@ -48,6 +48,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
   - Added timeout of 5 sec to tas api for user validation, and refactored to
     make validation more explicit in the absence of the old selected_identity
     notion ([#639](https://github.com/cyverse/atmosphere/pull/639))
+  - Linter runs more strict. Many changes were made to satisfy linter.
+    ([#664](https://github.com/cyverse/atmosphere/pull/664))
 
 ### Removed
   - Remove code/vars related to old allocation system

api/base/views/__init__.py

@@ -1 +1,3 @@
 from .version import VersionViewSet, DeployVersionViewSet
+
+__all__ = ["VersionViewSet", "DeployVersionViewSet"]

api/exceptions.py

@@ -136,7 +136,6 @@ def over_allocation(allocation_exception):
 
 
 def member_action_forbidden(username, object_type, object_id):
-    message = "User %s does not have permission to act on %s %s. Make sure that you are a Leader of the Project that %s %s resides in." % (username, object_type, object_id, object_type, object_id)
-    return failure_response(
-        status.HTTP_403_FORBIDDEN,
-        message)
+    message = "User %s does not have permission to act on %s %s. Make sure that you are a Leader of the Project that %s %s resides in." % (
+        username, object_type, object_id, object_type, object_id)
+    return failure_response(status.HTTP_403_FORBIDDEN, message)

api/tests/__init__.py

@@ -2,9 +2,7 @@
 Common functions used by all API test cases
 """
 from urlparse import urljoin
-from datetime import datetime
 from rest_framework import status
-from time import sleep
 
 
 def verify_expected_output(test_client, api_out, expected_out):

api/tests/factories/__init__.py

@@ -18,3 +18,14 @@
 from .allocation_source_factory import AllocationSourceFactory, UserAllocationSourceFactory
 from .boot_script_factory import BootScriptRawTextFactory
 from .instance_source_factory import InstanceSourceFactory
+
+__all__ = [
+    "TagFactory", "UserFactory", "AnonymousUserFactory", "ProviderFactory",
+    "GroupFactory", "GroupMembershipFactory", "ProjectFactory",
+    "IdentityFactory", "IdentityMembershipFactory", "ImageFactory",
+    "InstanceFactory", "InstanceHistoryFactory", "InstanceStatusFactory",
+    "ApplicationVersionFactory", "QuotaFactory", "ProviderTypeFactory",
+    "ProviderMachineFactory", "PlatformTypeFactory", "SizeFactory",
+    "AllocationSourceFactory", "UserAllocationSourceFactory",
+    "BootScriptRawTextFactory", "BootScriptURLFactory", "InstanceSourceFactory"
+]

api/tests/factories/boot_script_factory.py

@@ -1,6 +1,6 @@
 import factory
 from factory import fuzzy
-from api.tests.factories import UserFactory, InstanceFactory
+from api.tests.factories import UserFactory
 from core.models import BootScript, ScriptType
 
 

api/tests/factories/instance_factory.py

@@ -3,7 +3,6 @@
 from django.utils import timezone
 from .user_factory import UserFactory
 from .project_factory import ProjectFactory
-from .instance_source_factory import InstanceSourceFactory
 from .provider_machine_factory import ProviderMachineFactory
 from .identity_factory import IdentityFactory
 

api/tests/factories/instance_source_factory.py

@@ -1,6 +1,6 @@
 import factory
 import uuid
-from core.models import ProviderMachine, InstanceSource
+from core.models import InstanceSource
 from .provider_factory import ProviderFactory
 
 class InstanceSourceFactory(factory.DjangoModelFactory):

api/tests/factories/provider_machine_factory.py

@@ -1,11 +1,8 @@
 import factory
-import uuid
-from core.models import ProviderMachine, InstanceSource
+from core.models import ProviderMachine
 from .version_factory import ApplicationVersionFactory
 from .image_factory import ImageFactory
-from .provider_factory import ProviderFactory
 from .instance_source_factory import InstanceSourceFactory
-from .version_factory import ApplicationVersionFactory
 
 
 class ProviderMachineFactory(factory.DjangoModelFactory):

api/tests/test_auth.py

@@ -82,7 +82,10 @@ def test_valid_ldap_auth(self, patch_func):
         response = self.client.post(self.auth_url, data)
         resp_data = response.data
         self.assertEquals(response.status_code, 201)
-        self.assertTrue(resp_data['username'] == self.username, "Response returned unexpected username <%s>, expected %s" % (resp_data['username'], self.username))
+        self.assertTrue(
+            resp_data['username'] == self.username,
+            "Response returned unexpected username <%s>, expected %s" %
+            (resp_data['username'], self.username))
         self.assertTrue(resp_data['token'] is not None)
 
     @override_settings(AUTHENTICATION_BACKENDS=('django_cyverse_auth.authBackends.LDAPLoginBackend',))

api/tests/v1/test_profile.py

@@ -21,7 +21,7 @@ def test_external_accounts_are_created(self):
         force_authenticate(request, user=user)
 
         with mock.patch("api.v1.views.profile.create_new_accounts") as mock_create_new_accounts:
-            response = view(request)
+            view(request)
             mock_create_new_accounts.assert_called_once()
 
     @override_settings(AUTO_CREATE_NEW_ACCOUNTS=True)
@@ -40,5 +40,5 @@ def test_external_accounts_are_not_created_for_invalid_user(self):
         with mock.patch.object(AtmosphereUser, 'is_valid', return_value=False), \
                 mock.patch("api.v1.views.profile.create_new_accounts") \
                 as mock_create_new_accounts:
-            response = view(request)
+            view(request)
             mock_create_new_accounts.assert_not_called()

api/tests/v2/test_access_tokens.py

@@ -1,12 +1,8 @@
-import requests
-
 from rest_framework.test import APITestCase, APIRequestFactory, force_authenticate
-from django.core.urlresolvers import reverse
 
 from api.v2.views import AccessTokenViewSet
 from api.tests.factories import UserFactory, AnonymousUserFactory
-from core.models.access_token import AccessToken, create_access_token
-
+from core.models.access_token import create_access_token
 from .base import APISanityTestCase
 
 
@@ -98,7 +94,7 @@ def test_edit_not_public(self):
 
     def test_delete(self):
         force_authenticate(self.delete_request, user=self.user)
-        delete_response = self.delete_view(self.delete_request, pk=self.access_token.id)
+        self.delete_view(self.delete_request, pk=self.access_token.id)
         force_authenticate(self.list_request, user=self.user)
         list_response = self.list_view(self.list_request)
         data = list_response.data.get('results')

api/tests/v2/test_instance_actions.py

@@ -4,13 +4,11 @@
 from django.core.urlresolvers import reverse
 from django.utils import timezone
 
-from rest_framework.test import APIClient
-
 from rest_framework.test import APITestCase, APIRequestFactory, force_authenticate
 from api.tests.factories import (
-    UserFactory, AnonymousUserFactory, InstanceFactory, InstanceHistoryFactory, InstanceStatusFactory,
-    ApplicationVersionFactory, ProviderMachineFactory, IdentityFactory, ProviderFactory,
-    SizeFactory, AllocationSourceFactory)
+    UserFactory, AnonymousUserFactory, InstanceFactory,
+    InstanceHistoryFactory, InstanceStatusFactory, ProviderMachineFactory,
+    IdentityFactory, ProviderFactory, SizeFactory, AllocationSourceFactory)
 from api.v2.views import InstanceViewSet
 from core.models import (
     InstanceAllocationSourceSnapshot,

api/tests/v2/test_instances.py

@@ -6,14 +6,13 @@
 
 from rest_framework.test import APIClient
 
-from rest_framework.test import APITestCase, APIRequestFactory, force_authenticate
+from rest_framework.test import APITestCase
 from api.tests.factories import (
-    GroupFactory, UserFactory, AnonymousUserFactory, InstanceFactory, InstanceHistoryFactory, InstanceStatusFactory, SizeFactory,
-    ImageFactory, ApplicationVersionFactory, InstanceSourceFactory, ProviderMachineFactory, IdentityFactory, ProviderFactory,
-    IdentityMembershipFactory, QuotaFactory)
+    UserFactory, AnonymousUserFactory, InstanceFactory,
+    InstanceHistoryFactory, InstanceStatusFactory, ProviderMachineFactory,
+    IdentityFactory, ProviderFactory)
 from .base import APISanityTestCase
 from api.v2.views import InstanceViewSet
-from core.models import AtmosphereUser
 
 
 class InstanceTests(APITestCase, APISanityTestCase):

api/tests/v2/test_reporting.py

@@ -1,10 +1,9 @@
-import json
 from unittest import skip
 
 from django.core.urlresolvers import reverse
 from rest_framework.test import APITestCase, APIRequestFactory, force_authenticate
 
-from api.tests.factories import UserFactory, AnonymousUserFactory, InstanceFactory
+from api.tests.factories import UserFactory, AnonymousUserFactory
 from api.v2.views import ReportingViewSet
 from core.models import AtmosphereUser
 
@@ -26,74 +25,6 @@ def setUp(self):
         self.user = UserFactory.create()
         self.view = ReportingViewSet.as_view({'get': 'list'})
 
-    #def test_long_history_pull_excel_file(self):
-    #    """Will only work with a correct database."""
-    #    factory = APIRequestFactory()
-    #    url = '/api/v2/reporting?format=xlsx&start_date=2015-01-01&end_date=2017-01-28&provider_id=4&provider_id=5&provider_id=6'
-    #    request = factory.get(url)
-    #    sanity_user = AtmosphereUser.objects.get_by_natural_key('sgregory')
-    #    force_authenticate(request, user=sanity_user)
-    #    response = self.view(request)
-    #    self.assertEquals(response.status_code, 200)
-    #    self.assertEquals(response.accepted_media_type, 'application/vnd.ms-excel')
-    #    with open('/opt/dev/atmosphere/reporting.xlsx','wb') as reporting_file:
-    #        for chunk in response.rendered_content:
-    #            reporting_file.write(chunk)
-    #        reporting_file.flush()
-    #    return
-
-    #@skipUnless(contains_user('test-julianp'), 'The database does not contain the user test-julianp')
-    @skip('skip for now')
-    def test_a_sanity_check(self):
-        """Will only work with a correct database.
-        TODO: Create providers and fixtures necessary to get working.
-        """
-        factory = APIRequestFactory()
-        url = '/api/v2/reporting?start_date=2016-01-01&end_date=2016-10-25&provider_id=1&provider_id=2&provider_id=3&' \
-              'provider_id=4&provider_id=5&provider_id=6'
-        request = factory.get(url)
-        sanity_user = AtmosphereUser.objects.get_by_natural_key('test-julianp')
-        force_authenticate(request, user=sanity_user)
-        response = self.view(request)
-        self.assertEquals(response.status_code, 200)
-        self.assertEquals(len(response.data), 1)
-        received_data = json.loads(json.dumps(response.data, indent=2))
-        expected_data_json = '''[
-      {
-        "id": 29792,
-        "instance_id": "57259394-a1d2-4318-a0c0-5764f42db4be",
-        "username": "test-julianp",
-        "staff_user": "False",
-        "provider": "iPlant Workshop Cloud - Tucson",
-        "image_name": "Ubuntu 14.04.2 XFCE Base",
-        "version_name": "1.0",
-        "is_featured_image": true,
-        "hit_aborted": false,
-        "hit_active_or_aborted": 1,
-        "hit_active_or_aborted_or_error": 1,
-        "hit_active": true,
-        "hit_deploy_error": false,
-        "hit_error": false,
-        "size": {
-          "id": 105,
-          "uuid": "60fccf16-d0ba-488d-b8a5-46a9752dc2ca",
-          "url": "http://testserver/api/v2/sizes/60fccf16-d0ba-488d-b8a5-46a9752dc2ca",
-          "alias": "1",
-          "name": "tiny1",
-          "cpu": 1,
-          "disk": 0,
-          "mem": 4096,
-          "active": true,
-          "start_date": "2014-06-06T20:50:08.387646Z",
-          "end_date": null
-        },
-        "start_date": "09/20/16 17:36:57",
-        "end_date": null
-      }
-    ]'''
-        expected_data = json.loads(expected_data_json)
-        dict_eq_(self, received_data, expected_data)
-
     def test_is_not_public(self):
         factory = APIRequestFactory()
         url = reverse('api:v2:reporting-list')

api/tests/v2/test_sizes.py

@@ -2,7 +2,6 @@
     force_authenticate
 
 from django.core.urlresolvers import reverse
-from core.models import Size
 from api.v2.views import SizeViewSet
 from api.tests.factories import ProviderFactory, UserFactory,\
     AnonymousUserFactory, SizeFactory, IdentityFactory

api/v1/serializers/profile_serializer.py

@@ -1,7 +1,5 @@
 from core.models.profile import UserProfile
-from core.models import Identity
 from rest_framework import serializers
-from .identity_related_field import IdentityRelatedField
 
 
 class ProfileSerializer(serializers.ModelSerializer):

api/v1/views/identity.py

@@ -3,7 +3,6 @@
 
 from threepio import logger
 
-from core.models.group import Group
 from core.models.identity import Identity as CoreIdentity
 from core.models.provider import Provider
 

api/v1/views/identity_membership.py

@@ -7,7 +7,6 @@
 
 from core.models.group import Group, Identity
 from core.models import IdentityMembership as CoreIdentityMembership
-from core.query import only_current_provider
 
 from api import failure_response
 from api.v1.serializers import IdentitySerializer
@@ -49,16 +48,7 @@ def get(self, request, provider_uuid, identity_uuid, format=None):
         """
         Return the identity membership matching this provider+identity
         """
-        # Sanity checks:
-        # User is authenticated
-        user = request.user
         try:
-            # User is a member of a group ( TODO: loop through all instead)
-            group = user.group_set.get(name=user.username)
-            # Group has access to the identity on an active,
-            # currently-running provider
-            identity = group.current_identities.get(
-                                            uuid=identity_uuid)
             # All other members of the identity are visible
             id_members = CoreIdentityMembership.objects.filter(
                 identity__uuid=identity_uuid)

api/v1/views/instance.py

@@ -1,9 +1,11 @@
 from django.utils import timezone
 from django.db.models import Q
-
+from rtwo.exceptions import (
+    LibcloudInvalidCredsError, ConnectionFailure, LibcloudBadResponseError
+)
 from rest_framework import status
 from rest_framework.response import Response
-
+from socket import error as socket_error
 from threepio import logger
 
 from core.exceptions import ProviderNotActive
@@ -25,8 +27,6 @@
     SizeNotAvailable, HypervisorCapacityError, SecurityGroupNotCreated,
     VolumeAttachConflict, VolumeMountConflict, InstanceDoesNotExist,
     UnderThresholdError, ActionNotAllowed, Unauthorized,
-    # Technically owned by another
-    socket_error, ConnectionFailure, LibcloudInvalidCredsError, LibcloudBadResponseError
     )
 from service.instance import (
     run_instance_action,
@@ -491,7 +491,7 @@ def post(self, request, provider_uuid, identity_uuid, instance_id):
             return connection_failure(provider_uuid, identity_uuid)
         except ProviderNotActive as pna:
             return inactive_provider(pna)
-        except InstanceDoesNotExist as dne:
+        except InstanceDoesNotExist:
             return failure_response(
                 status.HTTP_404_NOT_FOUND,
                 'Instance %s no longer exists' % (instance_id,))
@@ -751,7 +751,7 @@ def delete(self, request, provider_uuid, identity_uuid, instance_id):
             return connection_failure(provider_uuid, identity_uuid)
         except LibcloudInvalidCredsError:
             return invalid_creds(provider_uuid, identity_uuid)
-        except InstanceDoesNotExist as dne:
+        except InstanceDoesNotExist:
             return failure_response(
                 status.HTTP_404_NOT_FOUND,
                 "Instance %s does not exist" % instance_id)