Conversation
WalkthroughA new discovery mechanism for the Overseerr add-on was introduced by updating the configuration file and adding a new s6-rc one-shot service. The configuration file now includes a Changes
Sequence Diagram(s)sequenceDiagram
participant S6 as s6-rc (init)
participant Discovery as discovery/run script
participant Overseerr as Overseerr Service
participant HA as Home Assistant
S6->>Discovery: Start oneshot discovery service
Discovery->>Overseerr: Wait for port 5055 to be available
Overseerr-->>Discovery: Service becomes available
Discovery->>HA: Send discovery info (host, port) via bashio
Discovery->>S6: Log success or failure, exit
Suggested labels
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
|
Boop |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
|
Boop |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/run (1)
1-22: Discovery script implementation looks good with minor suggestionsThe script correctly:
- Waits for the Overseerr service to be available
- Sends discovery information to Home Assistant
- Handles success and error cases
A few observations:
- There's a typo in line 19: "send" should be "sent"
- bashio::log.info "Successfully send discovery information to Home Assistant." + bashio::log.info "Successfully sent discovery information to Home Assistant."
The port value has a "^" prefix in line 15. Is this intentional for bashio formatting?
The PR description mentioned possibly fetching the API key from settings.json, but this isn't implemented here. Was this intentionally omitted?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
overseerr/config.yaml(1 hunks)overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/run(1 hunks)overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/type(1 hunks)overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/up(1 hunks)
🔇 Additional comments (3)
overseerr/config.yaml (1)
18-19: Discovery configuration looks good!The addition of the
discoverykey withoverseerras a value properly enables the Home Assistant discovery mechanism for this add-on, which aligns with the PR objective.overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/type (1)
1-1: Appropriate service type configurationSetting this as a "oneshot" service is the correct approach for a discovery service that only needs to run once during container initialization.
overseerr/rootfs/etc/s6-overlay/s6-rc.d/discovery/up (1)
1-1: Proper s6 service execution pathThe "up" file correctly references the run script, following standard s6-overlay service configuration patterns.
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
Proposed Changes
Discover Overseerr integration
One thing we could consider is also fetching the api key from the integration using
jq '.main.apiKey' /config/settings.json. Not sure if that would be acceptable, but it would improve the set up even moreSummary by CodeRabbit