From 831c4c2fef65942aa9a0f57304c7ca717aa53e4c Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Mon, 27 Jun 2016 14:40:41 -0400 Subject: [PATCH] Cleaned up some more stale TODO's and FIXME's (or did / fixed the things) --- awx/api/serializers.py | 19 ++++--------------- awx/api/views.py | 1 - awx/main/migrations/_old_access.py | 2 -- awx/main/redact.py | 4 +--- awx/main/signals.py | 1 - awx/main/tasks.py | 4 ++-- 6 files changed, 7 insertions(+), 24 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 8ff163a4b6..78d3e32d03 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -218,8 +218,8 @@ class BaseSerializer(serializers.ModelSerializer): class Meta: fields = ('id', 'type', 'url', 'related', 'summary_fields', 'created', 'modified', 'name', 'description') - summary_fields = () # FIXME: List of field names from this serializer that should be used when included as part of another's summary_fields. - summarizable_fields = () # FIXME: List of field names on this serializer that should be included in summary_fields. + summary_fields = () + summarizable_fields = () # add the URL and related resources type = serializers.SerializerMethodField() @@ -668,11 +668,6 @@ class UnifiedJobStdoutSerializer(UnifiedJobSerializer): else: return super(UnifiedJobStdoutSerializer, self).get_types() - # TODO: Needed? - #def to_representation(self, obj): - # ret = super(UnifiedJobStdoutSerializer, self).to_representation(obj) - # return ret.get('result_stdout', '') - class UserSerializer(BaseSerializer): @@ -1309,7 +1304,6 @@ class InventorySourceOptionsSerializer(BaseSerializer): def validate_source_vars(self, value): # source_env must be blank, a valid JSON or YAML dict, or ... - # FIXME: support key=value pairs. try: json.loads((value or '').strip() or '{}') return value @@ -1335,9 +1329,9 @@ class InventorySourceOptionsSerializer(BaseSerializer): try: if source_script.organization != self.instance.inventory.organization: errors['source_script'] = "The 'source_script' does not belong to the same organization as the inventory." - except Exception: - # TODO: Log + except Exception as exc: errors['source_script'] = "'source_script' doesn't exist." + logger.error(str(exc)) if errors: raise serializers.ValidationError(errors) @@ -1610,8 +1604,6 @@ class ResourceAccessListElementSerializer(UserSerializer): class CredentialSerializer(BaseSerializer): - # FIXME: may want to make some fields filtered based on user accessing - class Meta: model = Credential fields = ('*', 'kind', 'cloud', 'host', 'username', @@ -1886,7 +1878,6 @@ class JobTemplateSerializer(UnifiedJobTemplateSerializer, JobOptionsSerializer): def validate_extra_vars(self, value): # extra_vars must be blank, a valid JSON or YAML dict, or ... - # FIXME: support key=value pairs. try: json.loads((value or '').strip() or '{}') return value @@ -2574,7 +2565,6 @@ class ScheduleSerializer(BaseSerializer): try: rrule.rrulestr(rrule_value) except Exception: - # TODO: Log raise serializers.ValidationError("rrule parsing failed validation.") return value @@ -2609,7 +2599,6 @@ class ActivityStreamSerializer(BaseSerializer): try: return json.loads(obj.changes) except Exception: - # TODO: Log logger.warn("Error deserializing activity stream json changes") return {} diff --git a/awx/api/views.py b/awx/api/views.py index 6daed99179..29c087b5ee 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2301,7 +2301,6 @@ class JobTemplateSurveySpec(GenericAPIView): try: obj.survey_spec = json.dumps(request.data) except ValueError: - # TODO: Log return Response(dict(error="Invalid JSON when parsing survey spec."), status=status.HTTP_400_BAD_REQUEST) if "name" not in obj.survey_spec: return Response(dict(error="'name' missing from survey spec."), status=status.HTTP_400_BAD_REQUEST) diff --git a/awx/main/migrations/_old_access.py b/awx/main/migrations/_old_access.py index 72bda1ee8d..ce952461ac 100644 --- a/awx/main/migrations/_old_access.py +++ b/awx/main/migrations/_old_access.py @@ -880,7 +880,6 @@ class JobTemplateAccess(BaseAccess): team_ids = Team.objects.filter(deprecated_users__in=[self.user]) - # TODO: I think the below queries can be combined deploy_permissions_ids = Permission.objects.filter( Q(user=self.user) | Q(team_id__in=team_ids), permission_type__in=allowed_deploy, @@ -1094,7 +1093,6 @@ class JobAccess(BaseAccess): allowed_check = [PERM_JOBTEMPLATE_CREATE, PERM_INVENTORY_DEPLOY, PERM_INVENTORY_CHECK] team_ids = Team.objects.filter(deprecated_users__in=[self.user]) - # TODO: I think the below queries can be combined deploy_permissions_ids = Permission.objects.filter( Q(user=self.user) | Q(team__in=team_ids), permission_type__in=allowed_deploy, diff --git a/awx/main/redact.py b/awx/main/redact.py index 10f7877faa..0e9ad0b5d1 100644 --- a/awx/main/redact.py +++ b/awx/main/redact.py @@ -29,12 +29,10 @@ class UriCleaner(object): username = o.username password = o.password - # Given a python MatchObject, with respect to redactedtext, find and + # Given a python MatchObject, with respect to redactedtext, find and # replace the first occurance of username and the first and second # occurance of password - # TODO: Ideally, we would replace username and password using the index - # that they were found at. uri_str = redactedtext[match.start():match.end()] if username: uri_str = uri_str.replace(username, UriCleaner.REPLACE_STR, 1) diff --git a/awx/main/signals.py b/awx/main/signals.py index 2a3a3d4b09..232a2edf02 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -322,7 +322,6 @@ def activity_stream_create(sender, instance, created, **kwargs): # Skip recording any inventory source directly associated with a group. if isinstance(instance, InventorySource) and instance.group: return - # TODO: Rethink details of the new instance object1 = camelcase_to_underscore(instance.__class__.__name__) changes = model_to_dict(instance, model_serializer_mapping) # Special case where Job survey password variables need to be hidden diff --git a/awx/main/tasks.py b/awx/main/tasks.py index c00da4e48d..f174ffe37f 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -142,8 +142,8 @@ def tower_periodic_scheduler(self): try: last_run = dateutil.parser.parse(fd.read()) return last_run - except Exception: - #TODO: LOG + except Exception as exc: + logger.error("get_last_run failed: {}".format(exc)) return None def write_last_run(last_run):