Skip to content
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

✨ ⚡️Change PUT to PATCH in all resource APIs and minor updates #173

Merged
merged 11 commits into from
Mar 15, 2018
7 changes: 3 additions & 4 deletions dataservice/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from config import config

from sqlalchemy.exc import IntegrityError
from werkzeug.exceptions import HTTPException


def create_app(config_name):
Expand Down Expand Up @@ -119,10 +118,10 @@ def register_error_handlers(app):
NB: Exceptions to be handled must be imported in the head of this module
"""
from dataservice.api import errors
app.register_error_handler(HTTPException, errors.http_error)
app.register_error_handler(IntegrityError, errors.integrity_error)
app.register_error_handler(404, errors.http_error)
app.register_error_handler(400, errors.http_error)
from werkzeug.exceptions import default_exceptions
for ex in default_exceptions:
app.register_error_handler(ex, errors.http_error)
Copy link
Contributor

Choose a reason for hiding this comment

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

No luck with HTTPException? It looks like default_exceptions is just all the subclasses of HTTPException, so you would think it would work.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we have to go with this approach for now. Doesn't look like the fix for the issue with HTTPException is in Flask 0.12. Its slated for Flask milestone 1.0 which isn't complete yet

pallets/flask#2314



def register_blueprints(app):
Expand Down
16 changes: 8 additions & 8 deletions dataservice/api/common/schemas.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
from dataservice.extensions import ma
from marshmallow import (
fields,
post_dump,
pre_dump,
validates_schema,
ValidationError
fields,
post_dump,
pre_dump,
validates_schema,
ValidationError
)
from flask import url_for, request
from flask import url_for
from dataservice.api.common.pagination import Pagination
from flask_marshmallow import Schema

Expand Down Expand Up @@ -120,8 +120,8 @@ class StatusSchema(Schema):
commit = fields.Str(description='API short commit hash', example='aef3b5a')
branch = fields.Str(description='API branch name', example='master')
tags = fields.List(
fields.String(description='Any tags associated with the version',
example=['rc', 'beta']))
fields.String(description='Any tags associated with the version',
example=['rc', 'beta']))

@post_dump(pass_many=False)
def wrap_envelope(self, data):
Expand Down
9 changes: 4 additions & 5 deletions dataservice/api/common/views.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import yaml
import jinja2
from flask.views import MethodView
from flask import render_template
from dataservice.api.common.schemas import (
response_generator,
paginated_generator
Expand Down Expand Up @@ -42,9 +41,9 @@ def register_spec(cls, spec):
for name, schema in c.schemas.items():
spec.definition(name, schema=schema)
ResponseSchema = response_generator(schema)
spec.definition(name+'Response', schema=ResponseSchema)
spec.definition(name + 'Response', schema=ResponseSchema)
PaginatedSchema = paginated_generator(schema)
spec.definition(name+'Paginated', schema=PaginatedSchema)
spec.definition(name + 'Paginated', schema=PaginatedSchema)

@staticmethod
def register_views(app):
Expand Down Expand Up @@ -80,7 +79,7 @@ def _format_docstring(func):
# No yaml section
if yaml_start == -1:
return
yaml_spec = yaml.safe_load(func.__doc__[yaml_start+3:])
yaml_spec = yaml.safe_load(func.__doc__[yaml_start + 3:])

# No template to insert
if 'template' not in yaml_spec:
Expand All @@ -95,7 +94,7 @@ def _format_docstring(func):
templated_spec.update(yaml_spec)

# Dump the deserialized spec back to yaml in the docstring
func.__doc__ = func.__doc__[:yaml_start+4]
func.__doc__ = func.__doc__[:yaml_start + 4]
func.__doc__ += yaml.dump(templated_spec, default_flow_style=False)
# The docstring is now ready for further processing by apispec

Expand Down
77 changes: 28 additions & 49 deletions dataservice/api/demographic/resources.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from flask import abort, request
from sqlalchemy.orm.exc import NoResultFound
from marshmallow import ValidationError

from dataservice.extensions import db
Expand Down Expand Up @@ -83,26 +82,16 @@ def get(self, kf_id):
resource:
Demographic
"""
dm = Demographic.query.get(kf_id)
if dm is None:
abort(404, 'could not find {} `{}`'
.format('demographic', kf_id))

# Get all
if kf_id is None:
d = Demographic.query.all()
return DemographicSchema(many=True).jsonify(d)
# Get one
else:
try:
d = Demographic.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
abort(404, 'could not find {} `{}`'
.format('demographic', kf_id))
return DemographicSchema().jsonify(d)

def put(self, kf_id):
"""
Update existing demographic
return DemographicSchema().jsonify(dm)

Update an existing demographic given a Kids First id
def patch(self, kf_id):
"""
Update an existing demographic. Allows partial update of resource
---
template:
path:
Expand All @@ -111,34 +100,25 @@ def put(self, kf_id):
resource:
Demographic
"""
body = request.json

# Check if demographic exists
try:
d1 = Demographic.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
abort(404, 'could not find {} `{}`'.format('demographic', kf_id))
dm = Demographic.query.get(kf_id)
if dm is None:
abort(404, 'could not find {} `{}`'
.format('demographic', kf_id))

# Validation only
# Partial update - validate but allow missing required fields
body = request.json or {}
try:
d = DemographicSchema(strict=True).load(body).data
# Request body not valid
except ValidationError as e:
abort(400, 'could not update demographic: {}'.format(e.messages))
dm = DemographicSchema(strict=True).load(body, instance=dm,
partial=True).data
except ValidationError as err:
abort(400, 'could not update demographic: {}'.format(err.messages))

# Deserialize
d1.external_id = body.get('external_id')
d1.race = body.get('race')
d1.gender = body.get('gender')
d1.ethnicity = body.get('ethnicity')
d1.participant_id = body.get('participant_id')

# Save to database
db.session.add(dm)
db.session.commit()

return DemographicSchema(200, 'demographic {} updated'
.format(d1.kf_id)).jsonify(d1), 200
return DemographicSchema(
200, 'demographic {} updated'.format(dm.kf_id)
).jsonify(dm), 200

def delete(self, kf_id):
"""
Expand All @@ -154,15 +134,14 @@ def delete(self, kf_id):
Demographic
"""
# Check if demographic exists
try:
d = Demographic.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
abort(404, 'could not find {} `{}`'.format('demographic', kf_id))
dm = Demographic.query.get(kf_id)
if dm is None:
abort(404, 'could not find {} `{}`'
.format('demographic', kf_id))

# Save in database
db.session.delete(d)
db.session.delete(dm)
db.session.commit()

return DemographicSchema(200, 'demographic {} deleted'
.format(d.kf_id)).jsonify(d), 200
.format(dm.kf_id)).jsonify(dm), 200
5 changes: 0 additions & 5 deletions dataservice/api/demographic/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@


class DemographicSchema(BaseSchema):
# Should not have to do this, since participant_id is part of the
# Demographic model and should be dumped. However it looks like this is
# still a bug in marshmallow_sqlalchemy. The bug is that ma sets
# dump_only=True for foreign keys by default. See link below
# https://github.com/marshmallow-code/marshmallow-sqlalchemy/issues/20
participant_id = field_for(Demographic, 'participant_id', required=True,
load_only=True)

Expand Down
65 changes: 25 additions & 40 deletions dataservice/api/diagnosis/resources.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from flask import abort, request
from sqlalchemy.orm.exc import NoResultFound
from marshmallow import ValidationError

from dataservice.extensions import db
Expand Down Expand Up @@ -84,19 +83,15 @@ def get(self, kf_id):
Diagnosis
"""
# Get one
try:
d = Diagnosis.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
dg = Diagnosis.query.get(kf_id)
if dg is None:
abort(404, 'could not find {} `{}`'
.format('diagnosis', kf_id))
return DiagnosisSchema().jsonify(d)
return DiagnosisSchema().jsonify(dg)

def put(self, kf_id):
def patch(self, kf_id):
"""
Update existing diagnosis

Update an existing diagnosis given a Kids First id
Update an existing diagnosis. Allows partial update of resource
---
template:
path:
Expand All @@ -105,34 +100,25 @@ def put(self, kf_id):
resource:
Diagnosis
"""
body = request.json
try:
# Check if diagnosis exists
d1 = Diagnosis.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
abort(404, 'could not find {} `{}`'.format('diagnosis', kf_id))
dg = Diagnosis.query.get(kf_id)
if dg is None:
abort(404, 'could not find {} `{}`'
.format('diagnosis', kf_id))

# Validation only
# Partial update - validate but allow missing required fields
body = request.json or {}
try:
d = DiagnosisSchema(strict=True).load(body).data
# Request body not valid
except ValidationError as e:
abort(400, 'could not update diagnosis: {}'.format(e.messages))
dg = DiagnosisSchema(strict=True).load(body, instance=dg,
partial=True).data
except ValidationError as err:
abort(400, 'could not update diagnosis: {}'.format(err.messages))

# Deserialize
d1.external_id = body.get('external_id')
d1.diagnosis = body.get('diagnosis')
d1.diagnosis_category = body.get('diagnosis_category')
d1.tumor_location = body.get('tumor_location')
d1.age_at_event_days = body.get('age_at_event_days')
d1.participant_id = body.get('participant_id')

# Save to database
db.session.add(dg)
db.session.commit()

return DiagnosisSchema(200, 'diagnosis {} updated'
.format(d1.kf_id)).jsonify(d1), 200
return DiagnosisSchema(
200, 'diagnosis {} updated'.format(dg.kf_id)
).jsonify(dg), 200

def delete(self, kf_id):
"""
Expand All @@ -149,15 +135,14 @@ def delete(self, kf_id):
"""

# Check if diagnosis exists
try:
d = Diagnosis.query.filter_by(kf_id=kf_id).one()
# Not found in database
except NoResultFound:
abort(404, 'could not find {} `{}`'.format('diagnosis', kf_id))
dg = Diagnosis.query.get(kf_id)
if dg is None:
abort(404, 'could not find {} `{}`'
.format('diagnosis', kf_id))

# Save in database
db.session.delete(d)
db.session.delete(dg)
db.session.commit()

return DiagnosisSchema(200, 'diagnosis {} deleted'
.format(d.kf_id)).jsonify(d), 200
.format(dg.kf_id)).jsonify(dg), 200
6 changes: 0 additions & 6 deletions dataservice/api/diagnosis/schemas.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from marshmallow_sqlalchemy import field_for
from marshmallow import validates, ValidationError

from dataservice.api.diagnosis.models import Diagnosis
from dataservice.api.common.schemas import BaseSchema
Expand All @@ -8,11 +7,6 @@


class DiagnosisSchema(BaseSchema):
# Should not have to do this, since participant_id is part of the
# Diagnosis model and should be dumped. However it looks like this is
# still a bug in marshmallow_sqlalchemy. The bug is that ma sets
# dump_only=True for foreign keys by default. See link below
# https://github.com/marshmallow-code/marshmallow-sqlalchemy/issues/20
participant_id = field_for(Diagnosis, 'participant_id', required=True,
load_only=True, example='DZB048J5')
age_at_event_days = field_for(Diagnosis, 'age_at_event_days',
Expand Down
2 changes: 0 additions & 2 deletions dataservice/api/docs/resources.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import os
import glob
import subprocess
from flask.views import View
from flask import jsonify, current_app, render_template

Expand Down
Loading