Add disconnect feature to api #4
Conversation
- Implemented DELETE /api/disconnect/{publisher_id} endpoint for disconnecting active publishing operations by publisher ID.
- Updated handler to use publisher_id directly instead of player_id.
- Improved permission checks and error handling for disconnect operation.
- Refactored code for clarity and RESTful compliance.
- Remove all player_id and player-to-publisher mapping logic from the interface. - Update disconnect_publisher and generate_json_for_publisher signatures to use publisher_id directly. - Remove find_publisher_by_player_key declaration. - Clarify API for publisher-based operations.
- Remove all player_id and player-to-publisher mapping logic from implementation. - Update disconnect_publisher and generate_json_for_publisher to use publisher_id directly. - Remove find_publisher_by_player_key and related code. - Fix all syntax and brace issues from refactor. - Ensure all publisher operations are consistent and robust.
- Document new API endpoint for disconnecting publishers using publisher_id. - Remove references to player_id and player-to-publisher mapping. - Clarify usage and expected parameters for disconnect operation. - Ensure documentation matches refactored backend logic.
servusrene
left a comment
servusrene
left a comment
There was a problem hiding this comment.
It looks quite good already, there are still some minor inconsistencies in the code.
| } | ||
|
|
||
| // Validate player key and get mapped publisher key | ||
| char* mapped_publisher = find_publisher_by_player_key(const_cast<char*>(playerKey.c_str())); |
There was a problem hiding this comment.
we need the player to receive our stats, so why have you deleted the code?
|
|
||
| } | ||
|
|
||
| char* CSLSManager::find_publisher_by_player_key(char *player_key) { |
There was a problem hiding this comment.
same as in line 244: we need the player to receive our stats, so why have you deleted the code?
- Rename parameter from player_key to publisher_key in disconnect_publisher method. - Update logging messages to reflect the change from player_key to publisher_key.
…ine endings for cross-platform compatibility - Enhance trim and replace logic to ensure config files are parsed correctly regardless of OS or editor - Improve error logging and block parsing for better maintainability - Prevent parsing errors caused by stray whitespace, tabs, or comment lines - Ensures reliable config loading in Docker and all deployment environments
- Update disconnect logic to support both publisher ID and player ID. - Refactor generate_json_for_publisher to accept player key instead of publisher ID. - Add find_publisher_by_player_key method to resolve player keys to publisher IDs. - Improve logging for publisher disconnection and JSON generation processes.
|
@servusrene Some code was unintentionally removed in the previous changes. I’ve restored it so players can continue receiving stats and corrected the variable name in disconnect_publisher. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Docstrings generation was requested by @servusrene. * #4 (comment) The following files were modified: * `slscore/SLSApiServer.cpp` * `slscore/SLSManager.cpp` * `slscore/conf.cpp`
|
Hi @servusrene, I’ve addressed the review feedback in PR #4:
The PR is currently shown as mergeable, but it still has a “changes requested” review, so I’m guessing that might be what’s blocking the merge. Is there anything else you’d like me to adjust before it can be merged? Thanks! |
|
Hey @abdulkadirozyurt, Please excuse the long wait. I am currently working behind the scenes on an improved version of the srt-live server. Among other things, the API is being optimized. This means that this PR would not be 100% compatible right away. As soon as the version is released, I will merge this feature with the adjustments for the new API function. However, I can't say at this point when the new version will be released. |
|
Her @servusrene |
2 participants
Hi!
Sorry about that. I had to delete my fork.
I created a new pull request for this.
Summary by CodeRabbit
New Features
Documentation
Chores