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

Sharks - Morgan B. and Huma H. #21

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@
def create_app(test_config=None):
app = Flask(__name__)

from .routes import planets_bp
app.register_blueprint(planets_bp)

return app
50 changes: 49 additions & 1 deletion app/routes.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,50 @@
from flask import Blueprint
from flask import Blueprint, jsonify, abort, make_response

planets_bp = Blueprint("planets", __name__, url_prefix="/planets")

class Planet:
def __init__ (self, id, name, description, moon_count):
self.id = id
self.name = name
self.description = description
self.moon_count = moon_count

def to_json(self):
return {
"id": self.id,
"name": self.name,
"description": self.description,
"moon_count": self.moon_count
}

planets = [
Planet(1, "Mercury", "red planet", 1),
Planet(2, "Venus", "blue planet", 3),
Planet(1, "Earth", "water planet", 1)
]

def validate_planet(id):
try:
id = int(id)
except:
abort(make_response({"message": f"Planet {id} is not valid"}, 400))

for planet in planets:
if planet.id == id:
return planet

return abort(make_response({"message": f"Planet {id} not found"}, 404))

Choose a reason for hiding this comment

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

LGTM 👍🏽


@planets_bp.route("", methods=["GET"])
def get_planets():

Choose a reason for hiding this comment

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

Nice job naming the method to describe what this route does.

In the future, when you split your routes up into distinct route files (cat_routes.py and dog_routes.py for example in a routes directory) then you could have method names like get_all() and get_one() since we know that all the routes in cat_routes.py are related to the Cat class.

planet_response_body = []
for planet in planets:
planet_response_body.append(planet.to_json())

return jsonify(planet_response_body)

Choose a reason for hiding this comment

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

Adding 200 status code adds clarity even though it happens by default

return jsonify(planet_response_body), 200


@planets_bp.route("/<planet_id>", methods=["GET"])
def read_one_planet(planet_id):
planet = validate_planet(planet_id)

return jsonify(planet.to_json())

Choose a reason for hiding this comment

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

Same comment as above, you can return status code 200 here

2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ itsdangerous==1.1.0
Jinja2==2.11.3
Mako==1.1.4
MarkupSafe==1.1.1
psycopg2-binary==2.8.6
psycopg2-binary==2.9.3
pycodestyle==2.6.0
pytest==6.2.3
pytest-cov==2.12.1
Expand Down