Skip to content

Conversation

@vpetrovykh
Copy link
Contributor

This is a draft because some of the QB code here is bugged.

@vpetrovykh vpetrovykh requested review from 1st1 and elprans July 19, 2025 02:14
@github-actions
Copy link

github-actions bot commented Jul 19, 2025

Docs preview deploy

✅ Successfully deployed docs preview for commit c100f03:

https://edgedb-docs-mlteks0s3-edgedb.vercel.app

(Last updated: Jul 19, 2025, 02:18:09 UTC)

Copy link
Contributor

@scotttrinh scotttrinh left a comment

Choose a reason for hiding this comment

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

Some general feedback about picking a more RESTful style

return team


To quite a team, we cannot just fetch the Person and set the ``team`` field to ``None``, because that field is computed and cannot be edited. Instead we need to fetch the team with the members list and remove the Person from there. The challenge is to do all this when given only the Person's name:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To quite a team, we cannot just fetch the Person and set the ``team`` field to ``None``, because that field is computed and cannot be edited. Instead we need to fetch the team with the members list and remove the Person from there. The challenge is to do all this when given only the Person's name:
To quit a team, we cannot just fetch the Person and set the ``team`` field to ``None``, because that field is computed and cannot be edited. Instead we need to fetch the team with the members list and remove the Person from there. The challenge is to do all this when given only the Person's name:

Comment on lines 87 to 113
@app.post("/person/{pname}/add_friend", response_model=PersonWithFriends)
async def add_friend(
pname: str,
frname: str,
):
db = g.client
# fetch the main person
person = await db.get(
default.Person.select(
# fetch all properties
'*',
# also fetch friends (with properties)
friends=True,
).filter(
name=pname
)
)
# fetch the friend
friend = await db.get(
default.Person.filter(
name=frname
)
)
# append the new friend to existing friends
person.friends.append(friend)
await db.save(person)
return person
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to propose that we stick to a more basic RESTful style rather than having some RESTful endpoints, and some more generic RPC style endpoints like this one. So, my concrete suggestion for this case would be that add_friend is a POST to the friends linked resource:

Suggested change
@app.post("/person/{pname}/add_friend", response_model=PersonWithFriends)
async def add_friend(
pname: str,
frname: str,
):
db = g.client
# fetch the main person
person = await db.get(
default.Person.select(
# fetch all properties
'*',
# also fetch friends (with properties)
friends=True,
).filter(
name=pname
)
)
# fetch the friend
friend = await db.get(
default.Person.filter(
name=frname
)
)
# append the new friend to existing friends
person.friends.append(friend)
await db.save(person)
return person
@app.post("/person/{pname}/friends", response_model=list[Person])
async def add_friend(
pname: str,
frname: str,
):
db = g.client
# fetch the main person
person = await db.get(
default.Person.select(
# fetch all properties
'*',
# also fetch friends (with properties)
friends=True,
).filter(
name=pname
)
)
# fetch the friend
friend = await db.get(
default.Person.filter(
name=frname
)
)
# append the new friend to existing friends
person.friends.append(friend)
await db.save(person)
return person.friends

Comment on lines 201 to 207
class BaseTeam(default.Team.__variants__.Base):
name: default.Team.__typeof__.name
members: list[BasePerson]


@app.post("/teams/{team_name}/add_member", response_model=BaseTeam)
async def add_member(team_name: str, member_name: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

And similarly, something like:

Suggested change
class BaseTeam(default.Team.__variants__.Base):
name: default.Team.__typeof__.name
members: list[BasePerson]
@app.post("/teams/{team_name}/add_member", response_model=BaseTeam)
async def add_member(team_name: str, member_name: str):
class TeamMember(default.Person.__variants__.Base):
name: default.Person.__typeof__.name
@app.post("/teams/{team_name}/members", response_model=BaseTeam)
async def add_member(team_name: str, new_member: TeamMember):


.. code-block:: python

@app.post("/person/{pname}/quit_team", response_model=str)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid returning bare strings. Probably return the refetched person object.

Suggested change
@app.post("/person/{pname}/quit_team", response_model=str)
@app.delete("/person/{pname}/team", response_model=Person)

Comment on lines 260 to 271
@app.get("/games/", response_model=list[GameSessionBase])
async def get_games_with_team(team_name: str):
db = g.client
q = default.GameSession.filter(
# use an expression as a filter
lambda g: std.any(g.players.team.name == team_name),
# filter by status and is_full
status=default.GameStatus.Waiting,
is_full=False,
).order_by(title=True)

return await db.query(q)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would treat the "filter by team name" part here as just a special case of dynamic filtering, so this endpoint is really just get_games that allows you to filter on teams, status, fullness, or returns all games.

Suggested change
@app.get("/games/", response_model=list[GameSessionBase])
async def get_games_with_team(team_name: str):
db = g.client
q = default.GameSession.filter(
# use an expression as a filter
lambda g: std.any(g.players.team.name == team_name),
# filter by status and is_full
status=default.GameStatus.Waiting,
is_full=False,
).order_by(title=True)
return await db.query(q)
class GameFilter(BaseModel):
team_name: str | None = None
status: default.GameStatus | None = None
is_full: bool | None = None
@app.get("/games/", response_model=list[GameSessionBase])
async def get_games(game_filter: Annotated[GameFilter, Query()]):
db = g.client
q = default.GameSession.order_by(title=True)
if game_filter.team_name is not None:
q = q.filter(
# use an expression as a filter
lambda g: std.any(g.players.team.name == game_filter.team_name)
)
if game_filter.status is not None:
q = q.filter(status=game_filter.status)
if game_filter.is_full is not None:
q = q.filter(is_full=game_filter.is_full)
return await db.query(q)

Comment on lines 277 to 295
@app.post("/games/{game_id}/start", response_model=str)
async def start_game(game_id: uuid.UUID):
db = g.client
# instead of fetching the game, then updating and saving,
# we can update directly
q = default.GameSession.filter(
id=game_id,
# make sure the game is eligible to be started
status=default.GameStatus.Waiting,
is_full=True,
).update(
status='Active',
).select('*') # select the updated game
result = await db.query(q)

if len(result) == 0:
return "Game not started"
else:
return f"{result[0].num_players} player game started"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@app.post("/games/{game_id}/start", response_model=str)
async def start_game(game_id: uuid.UUID):
db = g.client
# instead of fetching the game, then updating and saving,
# we can update directly
q = default.GameSession.filter(
id=game_id,
# make sure the game is eligible to be started
status=default.GameStatus.Waiting,
is_full=True,
).update(
status='Active',
).select('*') # select the updated game
result = await db.query(q)
if len(result) == 0:
return "Game not started"
else:
return f"{result[0].num_players} player game started"
@app.post("/games/{game_id}/start", response_model=GameSession)
async def start_game(game_id: uuid.UUID):
db = g.client
# instead of fetching the game, then updating and saving,
# we can update directly
q = default.GameSession.filter(
id=game_id,
# make sure the game is eligible to be started
status=default.GameStatus.Waiting,
is_full=True,
).update(
status='Active',
).select('*') # select the updated game
result = await db.query(q)
if len(result) == 0:
raise HTTPException(status_code=404, f"No eligible game found with id '{game_id}'')
return result

Comment on lines 301 to 311
@app.get("/games/fit_team", response_model=list[GameSessionBase])
async def get_games_for_team(team_name: str):
db = g.client
# make the team subquery
team = default.Team.filter(name=team_name)
# use the team subquery to find the games
q = default.GameSession.filter(
lambda g: g.max_players - g.num_players >= std.count(team.members),
).order_by(title=True)

return await db.query(q) No newline at end of file
Copy link
Contributor

@scotttrinh scotttrinh Jul 23, 2025

Choose a reason for hiding this comment

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

Oh, this is fancy! Putting on my RESTifarian glasses, I'd write this as a fit_team_name attribute on the GameFilter from earlier. We'd also want to update it to take into account games that already have some members of this team, so the math is a bit less straightforward than the current algorithm.

This primer focuses on integration between FastAPI and Gel in ORM/QB
mode.
Change the examples to use Person instead of User to avoid implication
that this has anything to do with authentication and user accounts.

Some markup cleanup.
@gel-data-cla
Copy link

gel-data-cla bot commented Sep 25, 2025

The following commit authors need to sign the Contributor License Agreement:

  • 49699333+dependabot[bot]@users.noreply.github.com

Click the button to sign:
CLA not signed

@vpetrovykh vpetrovykh mentioned this pull request Sep 25, 2025
@vpetrovykh vpetrovykh changed the base branch from qb-primer to master September 25, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants