Update openapi #4
Conversation
Karolk99
force-pushed
the
update-openapi
branch
from
9737a8e
to
14b64a9
Compare
|
What about the failing tests? There have been a lot of updates in the openapi, so we should make some updates in the client? |
First of all, I fixed GitHub workflow and current tests (9) are failing |
Karolk99
force-pushed
the
update-openapi
branch
4 times, most recently
from
f9494bc
to
9e0e6e0
Compare
| "Room", | ||
| "RoomConfig", | ||
| "RoomConfigVideoCodec", | ||
| "Peer", |
There was a problem hiding this comment.
Why are we removing Peer and PeerStatus? Isn't it needed when rendering the response when getting all rooms?
There was a problem hiding this comment.
Also PeerToken, Room (from _room_api) are missing
There was a problem hiding this comment.
Right now all types are imported under fishjam namespace
fishjam/api/_room_api.py
Outdated
| """Returns list of all rooms""" | ||
|
|
||
| return self._request(room_get_all_rooms).data | ||
| def create_room(self, options: RoomOptions = RoomOptions()) -> Room: |
There was a problem hiding this comment.
I think we should be passing the room options directly in the create_room method, instead of passing config.
This doesn't require creating the RoomOptions object each time we create room. Also, options are the only argument to the create_room method, so using RoomOptions seems redundant.
There was a problem hiding this comment.
After our conversation with @AHGIJMKLKKZNPJKQR, we decide to leave as it is.
|
I know that we will likely update README's for all projects later, but I think we can update slightly it in this PR, e.g. remove example of adding component. |
I'm planning to rewrite the whole README in this PR but first I want to have accepted SKD API |
Karolk99
force-pushed
the
update-openapi
branch
from
1979cd4
to
1134054
Compare
Karolk99
force-pushed
the
update-openapi
branch
from
25d5cd7
to
10bfacc
Compare
|
|
||
|
|
||
| @dataclass | ||
| class Info: |
There was a problem hiding this comment.
Maybe Resource or Entity
| class Info: | |
| class Resource: |
| peers = map(lambda peer: peer.id == peer_access.peer.id, room.peers) | ||
| return bool(peers) |
There was a problem hiding this comment.
This is always true if there are any peers in a room bool([False, False]) = True.
| peers = map(lambda peer: peer.id == peer_access.peer.id, room.peers) | |
| return bool(peers) | |
| return any(peer.id == peer_access.peer.id for peer in room.peers) |
| if not peer_access or not peer_in_room: | ||
| return self.__create_peer(room_name, username) | ||
|
|
||
| self.logger.info("Peer and room exist: %s, %s", username, room_name) |
There was a problem hiding this comment.
nitpick: If room does exist we log room exists in the __find_or_create_room() and then we log peer and room exist. So we inform about room existence twice.
| room_name = None | ||
|
|
||
| for name, r_id in self.room_name_to_room_id.items(): | ||
| if room_id == r_id: | ||
| room_name = name | ||
| break | ||
|
|
||
| return room_name |
There was a problem hiding this comment.
| room_name = None | |
| for name, r_id in self.room_name_to_room_id.items(): | |
| if room_id == r_id: | |
| room_name = name | |
| break | |
| return room_name | |
| for name, r_id in self.room_name_to_room_id.items(): | |
| if room_id == r_id: | |
| return name | |
| return None |
fishjam/_ws_notifier.py
Outdated
| @@ -19,63 +32,64 @@ | |||
| ServerMessageSubscribeResponse, | |||
| ) | |||
|
|
|||
| ALLOWED_NOTIFICATION = ( | |||
There was a problem hiding this comment.
I think we should have a single place (module/ file) where the allowed notifications are stored - I don't see scenario where there are different ones for webhook and WsNotifier
Working on RoomManager example