Solar system final submition C22 Salma Anany and Tami Gaerter #18
Conversation
…ist of planet instances
wave 5 completed!!
… delete functions in planet_routes folder
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False | ||
|
|
||
| if config: | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get('SQLALCHEMY_DATABASE_URI') | ||
| app.config.update(config) | ||
| else: | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get('SQLALCHEMY_DATABASE_URI') |
There was a problem hiding this comment.
We're duplicating some code by having the same statement in either side of the if/else. When we see this, we can factor out the repeated code and just keep what's unique in the if/else:
app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
app.config['SQLALCHEMY_DATABASE_URI'] = os.environ.get('SQLALCHEMY_DATABASE_URI')
if config:
app.config.update(config)| # class Planet: | ||
| # def __init__(self,id, name, description, moon): | ||
| # self.id = id | ||
| # self.name = name | ||
| # self.description = description | ||
| # self.moon = moon | ||
| # | ||
| # planets = [ | ||
| # Planet(1, "Mercury", "the smallest planet in the solar system and orbits closest to the Sun, with extreme temperatures ranging from 430°C (800°F) during the day to -180°C (-290°F) at night.", 0), | ||
| # Planet(2, "Venus", "the Sun and has a thick, toxic atmosphere composed mostly of carbon dioxide, creating a runaway greenhouse effect that makes it the hottest planet in the solar system with surface temperatures around 465°C (870°F", 0), | ||
| # Planet(3, "Earth", "Our home planet, the only known planet to harbor life",1), | ||
| # Planet(4,"Mars", "Red Planet due to its reddish appearance caused by iron oxide (rust) on its surface. It has a thin atmosphere composed mostly of carbon dioxide", 2 ), | ||
| # Planet(4, "Pluto", " filled with icy bodies and other small objects. Once considered the ninth planet in the solar system, it was reclassified as a dwarf planet in 2006 due to its size and the fact that it hasn’t cleared its orbit of other debris. Pluto has a rocky core surrounded by a mantle of water ice and a thin atmosphere of nitrogen, methane, and carbon monoxide.", 5), | ||
| # Planet(5, "Jupiter","is the largest planet in the solar system, a gas giant composed primarily of hydrogen and helium, known for its Great Red Spot—a massive storm larger than Earth", 92), | ||
| # Planet(6, "Saturn","sixth planet from the Sun and is best known for its extensive and stunning ring system, made mostly of ice and rock particles. It is a gas giant composed primarily of hydrogen and helium" , 146), | ||
| # Planet(7, "Uranus","is the seventh planet from the Sun, an ice giant with a blue-green color due to the presence of methane in its atmosphere. It is unique for rotating on its side, with an extreme axial tilt of about 98 degrees", 27 ), | ||
| # Planet(8, "Neptune", "is a blue ice giant, the eighth planet from the Sun. Known for strong winds and dark storms", 14), | ||
| # Planet(9, "Pluto", " a small, icy world in the outer solar system. It has a diverse, frozen landscape featuring a distinctive heart-shaped region", 5 ), | ||
| # ] | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
If we are creating commits as we go, then all of our prior work can be revisited by looking at our code through our git commit history. This means we can and should delete out commented code that we are no longer using, since we can always refer back to that old code.
| name = request_body["name"] | ||
| moon= request_body["moon"] | ||
| description = request_body["description"] |
There was a problem hiding this comment.
What kind of error could be raised by this code? What's a way we could handle it?
| return response, 201 | ||
|
|
||
| @planets_bp.get("") | ||
| def get_all_planets(): |
There was a problem hiding this comment.
This function is starting to get long, some considerations could be if there are ways we could move some logic into helper functions or move constants to another file to keep the logic a little more concise.
| def validate_planet(planet_id, planets): | ||
| # Narrow the errors - data error, type error | ||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| abort(make_response({"message": f"Planet id {planet_id} invalid"}, 400)) | ||
|
|
||
| for planet in planets: | ||
| if planet.id == planet_id: | ||
| return planet | ||
|
|
||
| abort(make_response({"message": f"Planet {planet_id} not found"}, 404)) | ||
|
|
||
|
|
||
| def validate_planet_one(planet_id): | ||
|
|
||
| try: | ||
| planet_id = int(planet_id) | ||
| except: | ||
| response = {"message": f"planet {planet_id} invalid"} | ||
| abort(make_response(response, 400)) | ||
|
|
||
| query = db.select(Planet).where(Planet.id == planet_id) | ||
| planet = db.session.scalar(query) | ||
|
|
||
| if not planet: | ||
| response = {"message": f"{planet_id} not found"} | ||
| abort(make_response(response, 404)) | ||
|
|
||
| return planet |
There was a problem hiding this comment.
These two functions are duplicating work. How could we remove validate_planet and keep just the validate_planet_one function?
|
|
||
| @pytest.fixture | ||
| def update_existing_planet(app): | ||
| kronos= Planet(name ="Kronos", description = "Homeword of the Klingon Empire, rich in warrior culture and tradition, featuring magnificent cities built among dramatic mountain ranges, home to the legendary Great Hall of the High Council, and birthplace of many of the quadrant's greatest warriors and most honored traditions", moon = 7) |
There was a problem hiding this comment.
Large blocks of text can be hard for the eye to separate when moving quickly, it might be easier to read the individual attributes if they are listed on separate lines:
kronos= Planet(
name ="Kronos",
description = "Homeword of the Klingon Empire, rich in warrior culture and tradition, featuring magnificent cities built among dramatic mountain ranges, home to the legendary Great Hall of the High Council, and birthplace of many of the quadrant's greatest warriors and most honored traditions", moon = 7
)| return app.test_client() | ||
|
|
||
| @pytest.fixture | ||
| def get_every_single_planet(app): |
There was a problem hiding this comment.
Our fixture function names should describe the data they are setting up to be used in tests, because we want them to be flexible enough to be used in more than one test. It seems like this fixture might be named for a specific test based on the test_get_every_single_planet test. I would recommend renaming this something like save_five_planets, and it can be reused in any test where it would be useful to have 5 planets saved before the test starts.
| assert response.status_code == 404 | ||
| assert response_body == {"message": "10 not found"} | ||
|
|
||
| def test_get_one_planet_400(client, get_planet_invalid_id): |
There was a problem hiding this comment.
Since we want to start the test with an empty database, the only fixture we need is the client so we can make calls to our API.
def test_get_one_planet_400(client):| db.session.commit() | ||
|
|
||
| @pytest.fixture | ||
| def get_planet_invalid_id(app): |
There was a problem hiding this comment.
We wouldn't typically create a fixture for incorrect information. We want to create fixtures for the information we need stored in a database. If we are testing bad information coming from a request, we can set that up in the arrange step of our test when we are making the request using the client.
| db.session.commit() | ||
|
|
||
| @pytest.fixture | ||
| def update_existing_planet(app): |
There was a problem hiding this comment.
Does this fixture update an existing planet, or does it add a new planet to the database? How could we rename this function to better describe the actions it is taking?
Final submission on the Solar System API project.