DEV: Feature - Initial Buildup of Meeting Room Reservation API#1
DEV: Feature - Initial Buildup of Meeting Room Reservation API#1AliTahir-101 wants to merge 1 commit intodevfrom
Conversation
AliTahir-101
left a comment
AliTahir-101
left a comment
There was a problem hiding this comment.
@AliTahir-101 Please consider having a more structured code along with test coverage around your implementation. I've given a basic initial folder structure for an idea.
/project_root
/src
__init__.py
main.py # Application entry point, includes API routes
models.py # Pydantic models
services.py # Business logic, e.g., `BookingSystem`
settings.py # Configuration settings from the environment
/tests
test_main.py # Test cases for your application
.env # Environment variables
requirements.txt # Project dependencies
| @@ -0,0 +1,71 @@ | |||
| import logging | |||
There was a problem hiding this comment.
@AliTahir-101 Looks like the logging import isn't being used in the current implementation. Could we remove it to keep the codebase clean till you need it?
| from datetime import datetime | ||
| import requests | ||
| from fastapi import FastAPI | ||
| from pydantic import * |
There was a problem hiding this comment.
from pydantic import * is not a good practice. Can we please Import only what is needed here?
| @@ -0,0 +1,71 @@ | |||
| import logging | |||
| from datetime import datetime | |||
| import requests | |||
There was a problem hiding this comment.
The requests library is synchronous and will possibly block the event loop in your async function. Could we use some async alternative like httpx for async HTTP calls?
| app = FastAPI() | ||
|
|
||
|
|
||
| class RoomBooking(BaseModel): |
There was a problem hiding this comment.
It seems you missed the BaseModel import.
from pydantic import BaseModel.
|
|
||
|
|
||
| class BookingSystem: | ||
| url = "https://third-party-booking-system/room_availability" |
There was a problem hiding this comment.
We should have the base URL for the third-party booking system in the environment variable for flexibility and ease of configuration, especially when moving between different environments (development, staging, production) makes life easier 😊
| == "free" | ||
| ) | ||
|
|
||
| def book_rom(self, room_id: str, start_date: datetime, end_date: datetime) -> None: |
There was a problem hiding this comment.
same comments as the last function. Moreover, can we make it book_room so as to be consistent in using proper keywords?
| "end_date": end_date.isoformat(), | ||
| }, | ||
| ) | ||
| print("Room booked! 🎉" |
There was a problem hiding this comment.
We should use logging for any debugging or informational messages what do you think?
| ) | ||
|
|
||
|
|
||
| @app.post("/book_room") |
There was a problem hiding this comment.
We should change the endpoint @app.post("/book_room") to follow restful conventions, like @app.post("/rooms/book") what do you think?
| assert room_booking.start_date < room_booking.end_date | ||
| assert room_booking.start_date > datetime.now() |
There was a problem hiding this comment.
I think we should avoid assertions here and provide clear error responses with HTTPException.
from fastapi import HTTPException
from datetime import datetime
@app.post("/book_room")
async def book_room(room_booking: RoomBooking):
if room_booking.start_date >= room_booking.end_date:
raise HTTPException(status_code=400, detail="Start date must be before end date.")
if room_booking.start_date <= datetime.now():
raise HTTPException(status_code=400, detail="Start date must be in the future.")
| if available: | ||
| booking_system.book_rom( | ||
| room_booking.room_id, room_booking.start_date, room_booking.end_date | ||
| ) | ||
| return {"status": "success"} | ||
| else: | ||
| return {"status": "slot unavailable"} |
There was a problem hiding this comment.
@AliTahir-101 you should use Pydantic models for response bodies in FastAPI. Doing so ensures your responses are consistently structured and type-checked.
from pydantic import BaseModel
class BookingResponse(BaseModel):
status: str
message: str
@app.post("/book_room", response_model=BookingResponse)
async def book_room(room_booking: RoomBooking):
# ... your endpoint logic ...
return BookingResponse(status="success", message="Room booked successfully!")
There was a problem hiding this comment.
Moreover, add proper exception handling.
1 participant
Description
A team member has developed a new API designed for scheduling meeting rooms. Below is their inaugural PR submitted to the recently established repository. It requires a thorough review and constructive feedback.