Skip to content

Commit

Permalink
Add config to rules
Browse files Browse the repository at this point in the history
  • Loading branch information
ambrussimon committed Jan 22, 2018
1 parent 5a590db commit 837e00e
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 23 deletions.
10 changes: 5 additions & 5 deletions api/jobs/gears.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from __future__ import absolute_import

import bson.objectid
import copy
import datetime
from jsonschema import Draft4Validator, ValidationError
import gears as gear_tools
Expand Down Expand Up @@ -97,13 +98,13 @@ def suggest_for_files(gear, files):
return suggested_files

def validate_gear_config(gear, config_):
if len(gear.get('manifest', {}).get('config', {})) > 0:
invocation = gear_tools.derive_invocation_schema(gear['manifest'])
if len(gear.get('gear', {}).get('config', {})) > 0:
invocation = gear_tools.derive_invocation_schema(gear['gear'])
ci = gear_tools.isolate_config_invocation(invocation)
validator = Draft4Validator(ci)

try:
validator.validate(config_)
validator.validate(fill_gear_default_values(gear, config_))
except ValidationError as err:
key = None
if len(err.relative_path) > 0:
Expand All @@ -121,8 +122,7 @@ def fill_gear_default_values(gear, config_):
Given a gear and a config map, fill any missing keys using defaults from the gear's config
"""

if config_ is None:
config_ = {}
config_ = copy.deepcopy(config_) or {}

for k,v in gear['gear'].get('config', {}).iteritems():
if 'default' in v:
Expand Down
7 changes: 4 additions & 3 deletions api/jobs/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def post(self, cid):

validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True)
validate_regexes(doc)
get_gear(doc['gear_id'])
validate_gear_config(get_gear(doc['gear_id']), doc.get('config'))

doc['project_id'] = cid

Expand Down Expand Up @@ -231,8 +231,9 @@ def put(self, cid, rid):
updates = self.request.json
validate_data(updates, 'rule-update.json', 'input', 'POST', optional=True)
validate_regexes(updates)
if updates.get('gear_id'):
get_gear(updates['gear_id'])
gear_id = updates.get('gear_id', doc['gear_id'])
config_ = updates.get('config', doc.get('config'))
validate_gear_config(get_gear(gear_id), config_)

doc.update(updates)
config.db.project_rules.replace_one({'_id': bson.ObjectId(rid)}, doc)
Expand Down
4 changes: 2 additions & 2 deletions api/jobs/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def enqueue_job(job_map, origin, perm_check_uid=None):
if gear.get('gear', {}).get('custom', {}).get('flywheel', {}).get('invalid', False):
raise InputValidationException('Gear marked as invalid, will not run!')

config_ = fill_gear_default_values(gear, job_map.get('config', {}))
config_ = job_map.get('config', {})
validate_gear_config(gear, config_)

# Translate maps to FileReferences
Expand Down Expand Up @@ -197,7 +197,7 @@ def enqueue_job(job_map, origin, perm_check_uid=None):

# Config options are stored on the job object under the "config" key
config_ = {
'config': config_,
'config': fill_gear_default_values(gear, config_),
'inputs': { },
'destination': {
'type': destination.type,
Expand Down
3 changes: 3 additions & 0 deletions api/jobs/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ def create_potential_jobs(db, container, container_type, file_):

job = Job(str(gear['_id']), inputs, tags=['auto', gear_tag])

if 'config' in rule:
job.config = rule['config']

potential_jobs.append({
'job': job,
'rule': rule
Expand Down
2 changes: 2 additions & 0 deletions raml/schemas/definitions/rule.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"project_id": { "type": "string" },
"gear_id": { "type": "string" },
"name": { "type": "string" },
"config": { "type": "object" },
"any": { "$ref": "#/definitions/rule-items" },
"all": { "$ref": "#/definitions/rule-items" },
"disabled": { "type": "boolean" }
Expand All @@ -44,6 +45,7 @@
"_id": { "type": "string" },
"gear_id": { "type": "string" },
"name": { "type": "string" },
"config": { "type": "object" },
"any": { "$ref": "#/definitions/rule-items" },
"all": { "$ref": "#/definitions/rule-items" },
"disabled": { "type": "boolean" }
Expand Down
69 changes: 56 additions & 13 deletions tests/integration_tests/python/test_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,10 @@ def test_site_rules_copied_to_new_projects(randstr, data_builder, file_form, as_
data_builder.delete_group(group, recursive=True)


def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
def test_project_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
# create versioned gear to cover code selecting latest gear
gear = data_builder.create_gear(gear={'version': '0.0.1'})
gear_config = {'param': {'type': 'string', 'pattern': '^default|custom$', 'default': 'default'}}
gear = data_builder.create_gear(gear={'version': '0.0.1', 'config': gear_config})
project = data_builder.create_project()

bad_payload = {'test': 'rules'}
Expand Down Expand Up @@ -267,19 +268,42 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
'gear_id': '000000000000000000000000',
'name': 'csv-job-trigger-rule',
'any': [],
'all': [
{'type': 'file.type', 'value': 'tabular data'},
]
'all': [],
}

# try to add project rule w/ invalid rule-item (invalid type)
# NOTE this is a legacy rule
rule_json['all'] = [{'type': 'invalid', 'value': 'test'}]
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
assert r.status_code == 400
assert "'invalid' is not one of" in r.json()['message']

# try to add project rule w/ invalid rule-item (missing value)
# NOTE this is a legacy rule
rule_json['all'] = [{'type': 'file.name'}]
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
assert r.status_code == 400
assert "'value' is a required property" in r.json()['message']

# set valid rule-item
rule_json['all'] = [{'type': 'file.type', 'value': 'tabular data'}]

# try to add project rule w/ non-existent gear
# NOTE this is a legacy rule
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
assert r.status_code == 404

# add project rule w/ proper gear id
# try to add project rule w/ invalid config
# NOTE this is a legacy rule
rule_json['gear_id'] = gear
rule_json['config'] = {'param': 'invalid'}
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
assert r.status_code == 422
assert r.json()['reason'] == 'config did not match manifest'
del rule_json['config']

# add project rule w/ proper gear id
# NOTE this is a legacy rule
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
assert r.ok
rule = r.json()['_id']
Expand All @@ -305,10 +329,15 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
r = with_user.session.put('/projects/' + project + '/rules/' + rule, json={'gear_id': gear})
assert r.status_code == 403

# try to update rule to with invalid gear id
# try to update rule with invalid gear id
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'gear_id': '000000000000000000000000'})
assert r.status_code == 404

# try to update rule with invalid gear config
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'param': 'invalid'}})
assert r.status_code == 422
assert r.json()['reason'] == 'config did not match manifest'

# update name of rule
rule_name = 'improved-csv-trigger-rule'
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'name': rule_name})
Expand All @@ -323,11 +352,25 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
r = as_admin.post('/projects/' + project + '/files', files=file_form('test2.csv'))
assert r.ok

# test that job was created via rule
# test that job was created via rule and uses gear default config
gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
assert len(gear_jobs) == 1
assert len(gear_jobs[0]['inputs']) == 1
assert gear_jobs[0]['inputs'][0]['name'] == 'test2.csv'
assert gear_jobs[0]['config']['config'] == {'param': 'default'}

# update rule to have a custom config
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'param': 'custom'}})
assert r.ok

# upload another file that matches rule
r = as_admin.post('/projects/' + project + '/files', files=file_form('test3.csv'))
assert r.ok

# test that job was created via rule and custom config
gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
assert len(gear_jobs) == 2
assert gear_jobs[1]['config']['config'] == {'param': 'custom'}

# try to delete rule of non-existent project
r = as_admin.delete('/projects/000000000000000000000000/rules/000000000000000000000000')
Expand Down Expand Up @@ -368,7 +411,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a

# test that job was not created via rule
gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
assert len(gear_jobs) == 1 # still 1 from before
assert len(gear_jobs) == 2 # still 2 from before

# update test2.csv's metadata to include a valid measurement to spawn job
metadata = {
Expand Down Expand Up @@ -396,9 +439,9 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a

# test that only one job was created via rule
gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
assert len(gear_jobs) == 2
assert len(gear_jobs[1]['inputs']) == 1
assert gear_jobs[1]['inputs'][0]['name'] == 'test3.txt'
assert len(gear_jobs) == 3
assert len(gear_jobs[2]['inputs']) == 1
assert gear_jobs[2]['inputs'][0]['name'] == 'test3.txt'

# delete rule
r = as_admin.delete('/projects/' + project + '/rules/' + rule2)
Expand All @@ -423,7 +466,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a

# test that job was created via regex rule
gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
assert len(gear_jobs) == 3
assert len(gear_jobs) == 4

# delete rule
r = as_admin.delete('/projects/' + project + '/rules/' + rule3)
Expand Down

0 comments on commit 837e00e

Please sign in to comment.