Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for using a fishjam_id parameter as an alternative to providing a full fishjam_url when initializing Fishjam clients. The implementation allows clients to be created with either a complete URL or just an ID, which gets converted to a standardized URL format.
- Adds a new utility function to handle URL generation from either fishjam_id or fishjam_url
- Updates client constructors to accept fishjam_id as a primary parameter with fishjam_url as an optional fallback
- Modifies example code to use the new fishjam_id parameter instead of fishjam_url
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| fishjam/utils.py | New utility function to generate fishjam URL from ID or use provided URL |
| fishjam/errors.py | Custom exception for missing fishjam ID/URL validation |
| fishjam/api/_fishjam_client.py | Updated constructor to accept fishjam_id as primary parameter |
| fishjam/api/_client.py | Modified base client to use new URL generation logic |
| fishjam/_ws_notifier.py | Updated WebSocket notifier to support fishjam_id parameter |
| examples/room_manager/room_service.py | Example updated to use fishjam_id and removed unused url field |
| examples/room_manager/arguments.py | Command line arguments updated to support fishjam_id |
| .tool-versions | Added Python and Poetry version specifications |
Comments suppressed due to low confidence (1)
.tool-versions:2
- Poetry version 2.1.3 does not exist. The latest Poetry version as of early 2025 is 1.x series. Consider using a valid Poetry version like 1.8.3.
poetry 2.1.3
PiotrWodecki
approved these changes
Jul 29, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Fishjam ID as a part of the Connect API changes.
@AHGIJMKLKKZNPJKQR and @roznawsk feel free to suggest a better API