search

mpfmorawski / git-to-know-me

3 stars 1 forks source link

Stworzenie bazy dla czałego projektu #7

Closed mpfmorawski closed 2 years ago

mpfmorawski commented 2 years ago

Stworzenie bazy dla projektu:

pktiuk commented 2 years ago

Baza jest budowana na podstawie https://fastapi.tiangolo.com/tutorial/bigger-applications/#apirouter

Jażdy powinien u siebie bazować na klasie APIRouter, zamiast na FastAPI

from fastapi import APIRouter

router = APIRouter()

@router.get("/users/", tags=["users"])
async def read_users():
    return [{"username": "Rick"}, {"username": "Morty"}]
pktiuk commented 2 years ago

Apkę będzie można łatwo odpalić zarówno w wariancie zintegrowanym (wszystko na jednej instancji), jak i oddzielnym.

Mam jednak wciąż pewien problem natury technicznej w kontekście drugiego wariantu.
Jeśli teoretycznie uruchomilibyśmy naszą serwisy naszej aplikacji na wielu maszynach to w jakiś sposób powinniśmy utzymać gdzieś w kodzie informację o adresach maszyn na których można znaleźć poszczególne API.

Np serwis na serwerze A chcąc wykonać zapytanie do wybranego API powinien znać nie tylko nazwę endpointu (np /data/), ale także konkretny adres maszyny (i portu) gdzie serwis obsługujący dany endpoint jest dostępny.

Podobnie ma się sprawa, gdy chcemy odpalić równolegle wiele serwisów na jednej maszynie, wtedy każdy serwis otrzyma inny port, co też w jakiś sposób musimy ogarnąć.

#microservices routing

mpfmorawski commented 2 years ago

Mam uwagę do wszystkich osób zajmujących się backendem (@pktiuk, @lapankrz, @jprokopczuk) odnośnie kodu odpowiedzi HTTP.

Z FastAPI można zaimportować moduły status oraz HTTPException, które myślę, że warto stosować w naszym projekcie.

Przykładowo w drafcie pliku service_auth.py na branchu main zwracane jest 200. Zamiast tego proponuję przypisanie do status_code odpowiedniej wartości z zaimportowanego modułu status, czyli:

status_code=status.HTTP_200_OK

Analogicznie w pliku service_auth.py na branchu auth przy nieudanej próbie pobrania na ten moment zwracane jest return "No user with id: " + id + " exists in the database."

Proponuję zamiast tego skorzystać z HTTPException, a dokładniej:

raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User ID does not exist.")

Więcej info w dokumentacji FastAPI

pktiuk commented 2 years ago

Mam jeszcze jedną sprawę do osób pracujących nad backendem (@lapankrz, @jprokopczuk) dotyczy ona także dokumentacji.

Jak wiecie aplikacje FastAPI automatycznie generują dokumentację. Jest ona dostępna pod /docs
Jednak jakość tej dokumentacji zależy od tego, jak dobrze przygotujecie kod.

Dobrym przykładem jest ten endpoint.

obraz

Osoby piszące inne serwisy mogą łatwo zajrzeć i zobaczyć co dokładnie będzie zwracane.
Warto także opisywać parametry, które mogą być przekazywane (jeśli jakieś są)

Dobrze by było, gdyby wszystkie endpointy, które mają zwracać jakieś nieco bardziej złożone dane były w ten sposób udokumentowane.
Co o tym sądzisz @mpfmorawski jako PM oraz osoba odpowiedzialna za dokumentację?

mpfmorawski commented 2 years ago

W ostatnim tygodniu bawiłem się trochę FastAPI i też właśnie tą automatyczną dokumentacją (z myślą o dokumentacji naszego projektu). Sama w sobie oprócz tego, że wygląda dobrze, to jest jeszcze bardzo praktyczna. Nawet bez nakładu dodatkowego kodu ułatwia życie.

Jeśli chodzi o moje zdanie w tym temacie, to jestem jak najbardziej za. Myślałem, żeby poruszyć i ogarnąć ten temat lepiej przy kolejnym milestonie (po domknięciu MVP)