Skip to content

Commit

Permalink
Quality scoring calculation (#1132)
Browse files Browse the repository at this point in the history
Calculates quality score, adds to model and api endpoints. Part of #1097 
* added linter info to ImportTaskMessage model and api/v1/imports endpoints
* allow sending parameters through logger adapters
* update import task messages with content id
* improve importer output
* calculate and save scores for content and repo
* added rule sev and desc in importtaskmsg model and content api
* added task msg to content list api endpoint. changed matching to content.original_name
* Schema migration for content scoring, and data migration to set ContentRule values
  • Loading branch information
awcrosby authored Sep 18, 2018
1 parent 64838f8 commit 034f695
Show file tree
Hide file tree
Showing 18 changed files with 607 additions and 60 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ galaxy.min.css
.DS_Store
ansible-deployment/
yarn-error.log
galaxy/importer/galaxy-lint-rules/

# NOTE(cutwater): Temporary adding developer environment related files
# to .gitignore, however these files should not be included in project
Expand Down
33 changes: 31 additions & 2 deletions galaxy/api/serializers/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from django.core import urlresolvers as urls
from rest_framework import serializers
from collections import OrderedDict

from galaxy.main import models

Expand Down Expand Up @@ -96,7 +97,11 @@ class Meta:
'content_type',
'imported',
'download_count',
'role_type'
'role_type',
'quality_score',
'content_score',
'metadata_score',
'compatibility_score',
)

def get_platforms(self, instance):
Expand Down Expand Up @@ -124,6 +129,12 @@ def get_related(self, instance):
}

def get_summary_fields(self, instance):
# Support ansible-galaxy <= 2.6 by excluding unsupported messges
supported_types = ('INFO', 'WARNING', 'ERROR', 'SUCCESS', 'FAILED')
latest_task = models.ImportTask.objects.filter(
repository_id=instance.repository.id
).latest('id')

return {
'namespace': _NamespaceSerializer().to_representation(
instance.repository.provider_namespace.namespace),
Expand All @@ -132,7 +143,25 @@ def get_summary_fields(self, instance):
'content_type':
_ContentTypeSerializer().to_representation(
instance.content_type),
'dependencies': [str(g) for g in instance.dependencies.all()]
'dependencies': [str(g) for g in instance.dependencies.all()],
'task_messages': [
OrderedDict([
('id', m.id),
('message_type', m.message_type),
('message_text', m.message_text),
('content_id', m.content_id),
('is_linter_rule_violation', m.is_linter_rule_violation),
('linter_type', m.linter_type),
('linter_rule_id', m.linter_rule_id),
('rule_desc', m.rule_desc),
('rule_severity', m.rule_severity),
]) for m in models.ImportTaskMessage.objects.filter(
task_id=latest_task.id,
content_id=instance.id,
message_type__in=supported_types,
is_linter_rule_violation=True,
)
],
}


Expand Down
4 changes: 3 additions & 1 deletion galaxy/api/serializers/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ class Meta:
'readme_html',
'download_url',
'deprecated',
'community_score'
'community_score',
'quality_score',
)

def get_related(self, instance):
Expand Down Expand Up @@ -130,6 +131,7 @@ def get_summary_fields(self, instance):
'name': c.name,
'content_type': c.content_type.name,
'description': c.description,
'quality_score': c.quality_score,
}
for c in instance.content_objects.all()
]
Expand Down
1 change: 1 addition & 0 deletions galaxy/api/serializers/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def get_summary_fields(self, obj):
travis_build_url=obj.repository.travis_build_url,
format=obj.repository.format,
deprecated=obj.repository.deprecated,
quality_score=obj.repository.quality_score,
)
d['tags'] = [g.name for g in obj.tags.all()]
d['versions'] = [
Expand Down
6 changes: 5 additions & 1 deletion galaxy/api/serializers/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,11 @@ def get_summary_fields(self, obj):
summary['task_messages'] = [OrderedDict([
('id', g.id),
('message_type', g.message_type),
('message_text', g.message_text)
('message_text', g.message_text),
('content_id', g.content_id),
('is_linter_rule_violation', g.is_linter_rule_violation),
('linter_type', g.linter_type),
('linter_rule_id', g.linter_rule_id)
]) for g in obj.messages.filter(
message_type__in=supported_types).order_by('id')]
summary['role'] = {
Expand Down
44 changes: 34 additions & 10 deletions galaxy/common/logutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ class ImportTaskAdapter(logging.LoggerAdapter):
def __init__(self, logger, task_id):
super(ImportTaskAdapter, self).__init__(logger, {'task_id': task_id})

def process(self, msg, kwargs):
if self.extra:
if 'extra' not in kwargs:
kwargs.update({'extra': {}})
for key, value in self.extra.items():
kwargs['extra'][key] = value
return msg, kwargs


class ContentTypeAdapter(logging.LoggerAdapter):
def __init__(self, logger, content_type, content_name=None):
Expand All @@ -35,25 +43,41 @@ def __init__(self, logger, content_type, content_name=None):
})

def process(self, msg, kwargs):
if self.extra['content_name']:
prefix = '{}: {}'.format(
self.extra['content_type'].name,
self.extra['content_name'])
else:
prefix = self.extra['content_type']

msg = '[{}] {}'.format(prefix, msg)
if self.extra:
if 'extra' not in kwargs:
kwargs.update({'extra': {}})
for key, value in self.extra.items():
kwargs['extra'][key] = value
return msg, kwargs


class ImportTaskHandler(logging.Handler):
def emit(self, record):
# type: (logging.LogRecord) -> None
from galaxy.main import models
msg = self.format(record)

lint = {
'is_linter_rule_violation': False,
'linter_type': None,
'linter_rule_id': None,
'rule_desc': None,
'content_name': '',
}
if set(lint.keys()).issubset(vars(record).keys()):
lint['is_linter_rule_violation'] = record.is_linter_rule_violation
lint['linter_type'] = record.linter_type
lint['linter_rule_id'] = record.linter_rule_id
lint['rule_desc'] = record.rule_desc
lint['content_name'] = record.content_name

models.ImportTaskMessage.objects.using('logging').create(
task_id=record.task_id,
message_type=constants.ImportTaskMessageType.from_logging_level(
record.levelno).value,
message_text=msg,
message_text=record.msg,
is_linter_rule_violation=lint['is_linter_rule_violation'],
linter_type=lint['linter_type'],
linter_rule_id=lint['linter_rule_id'],
rule_desc=lint['rule_desc'],
content_name=lint['content_name'],
)
14 changes: 14 additions & 0 deletions galaxy/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ class ImportTaskState(Enum):
SUCCESS = 'SUCCESS'


class LinterType(Enum):
FLAKE8 = 'flake8'
YAMLLINT = 'yamllint'
ANSIBLELINT = 'ansible-lint'

@classmethod
def choices(cls):
return [
(cls.FLAKE8.value, 'flake8'),
(cls.YAMLLINT.value, 'yamllint'),
(cls.ANSIBLELINT.value, 'ansible-lint')
]


class RepositoryFormat(enum.Enum):
ROLE = 'role'
APB = 'apb'
Expand Down
6 changes: 3 additions & 3 deletions galaxy/importer/finders.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class ApbFinder(BaseFinder):
repository_format = constants.RepositoryFormat.APB

def find_contents(self):
self.log.info('Content search - Looking for file "apb.yml"')
self.log.debug('Content search - Looking for file "apb.yml"')
meta_path = _find_metadata(self.path, self.META_FILES)
if meta_path:
meta_path = os.path.join(self.path, meta_path)
Expand All @@ -81,7 +81,7 @@ class RoleFinder(BaseFinder):
repository_format = constants.RepositoryFormat.ROLE

def find_contents(self):
self.log.info(
self.log.debug(
'Content search - Looking for top level role metadata file')
meta_path = _find_metadata(self.path, ROLE_META_FILES)
if meta_path:
Expand All @@ -97,7 +97,7 @@ class FileSystemFinder(BaseFinder):
repository_format = constants.RepositoryFormat.MULTI

def find_contents(self):
self.log.info('Content search - Analyzing repository structure')
self.log.debug('Content search - Analyzing repository structure')
content = self._find_content()
# Check if finder has found at least one content
try:
Expand Down
57 changes: 57 additions & 0 deletions galaxy/importer/linters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def _check_files(self, paths):

class Flake8Linter(BaseLinter):

name = 'flake8'
cmd = 'flake8'

def _check_files(self, paths):
Expand All @@ -59,9 +60,28 @@ def _check_files(self, paths):
yield line.strip()
proc.wait()

def parse_id_and_desc(self, message):
try:
msg_parts = message.split(' ')
rule_desc = ' '.join(msg_parts[2:])

error_id = msg_parts[1]
if error_id[0] not in ['E', 'W']:
error_id = None

except IndexError:
error_id = None

if not error_id:
logger.error('No error_id found in {} message'.format(self.cmd))
return (None, None)

return (error_id, rule_desc)


class YamlLinter(BaseLinter):

name = 'yamllint'
cmd = 'yamllint'
config = os.path.join(LINTERS_DIR, 'yamllint.yaml')

Expand All @@ -73,9 +93,28 @@ def _check_files(self, paths):
yield line.strip()
proc.wait()

def parse_id_and_desc(self, message):
try:
msg_parts = message.split(' ')
rule_desc = ' '.join(msg_parts[2:])

error_id = msg_parts[1][1:-1]
if error_id not in ['error', 'warning']:
error_id = None

except IndexError:
error_id = None

if not error_id:
logger.error('No error_id found in {} message'.format(self.cmd))
return (None, None)

return (error_id, rule_desc)


class AnsibleLinter(BaseLinter):

name = 'ansible-lint'
cmd = 'ansible-lint'

def _check_files(self, paths):
Expand All @@ -102,3 +141,21 @@ def _check_files(self, paths):
# returncode 1 is app exception, 0 is no linter err, 2 is linter err
if proc.wait() not in (0, 2):
yield 'Exception running ansible-lint, could not complete linting'

def parse_id_and_desc(self, message):
try:
msg_parts = message.split(' ')
rule_desc = ' '.join(msg_parts[2:])

error_id = msg_parts[1][1:-1]
if error_id[0] not in ['E']:
error_id = None

except IndexError:
error_id = None

if not error_id:
logger.error('No error_id found in {} message'.format(self.cmd))
return (None, None)

return (error_id, rule_desc)
33 changes: 23 additions & 10 deletions galaxy/importer/loaders/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,35 @@ def load(self):
def lint(self):
if not self.linters:
return
self.log.info('Linting...')

linters = self.linters
if not isinstance(linters, (list, tuple)):
linters = [linters]

ok = True
all_linters_ok = True
for linter_cls in linters:
for message in linter_cls(self.root).check_files(self.rel_path):
message = '[%s] %s' % (linter_cls.cmd, message)
self.log.error(message)
ok = False

if ok:
self.log.info('Linting OK.')
return ok
linter_ok = True
linter_obj = linter_cls(self.root)
for message in linter_obj.check_files(self.rel_path):
if linter_ok:
self.log.info('{} Warnings:'.format(linter_obj.name))
linter_ok = False
error_id, rule_desc = linter_obj.parse_id_and_desc(message)
if error_id:
extra = {
'is_linter_rule_violation': True,
'linter_type': linter_cls.cmd,
'linter_rule_id': error_id,
'rule_desc': rule_desc
}
self.log.warning(message, extra=extra)
else:
self.log.warning(message)
all_linters_ok = False
if linter_ok:
self.log.info('{} OK.'.format(linter_obj.name))

return all_linters_ok

# FIXME(cutwater): Due to current object model current object limitation
# this leads to copying README file over multiple roles.
Expand Down
7 changes: 3 additions & 4 deletions galaxy/importer/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ def load(self):
result = list(self._load_contents(contents))
readme = self._get_readme(finder.repository_format)

if not all(v[1] for v in result):
self.log.error('Lint failed')
# raise exc.ContentLoadError('Lint failed')

name = None
if (finder.repository_format in (constants.RepositoryFormat.ROLE,
constants.RepositoryFormat.APB)
Expand Down Expand Up @@ -113,7 +109,10 @@ def _load_contents(self, contents):
loader = loader_cls(content_type, rel_path, self.path,
logger=self.log, **extra)
content = loader.load()
self.log.info('===== LINTING {}: {} ====='.format(
content_type.name, content.name))
lint_result = loader.lint()
self.log.info(' ')
yield content, lint_result

def _get_readme(self, repository_format):
Expand Down
5 changes: 5 additions & 0 deletions galaxy/main/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,13 @@ class ContentBlockAdmin(admin.ModelAdmin):
pass


class ContentRuleAdmin(admin.ModelAdmin):
pass


admin.site.register(models.Platform, PlatformAdmin)
admin.site.register(models.CloudPlatform, CloudPlatformAdmin)
admin.site.register(models.Content, RoleAdmin)
admin.site.register(models.RepositoryVersion, RepositoryVersionAdmin)
admin.site.register(models.ContentBlock, ContentBlockAdmin)
admin.site.register(models.ContentRule, ContentRuleAdmin)
Loading

0 comments on commit 034f695

Please sign in to comment.