-
Notifications
You must be signed in to change notification settings - Fork 4
Add addon availability API #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
612c1b8 to
184d6da
Compare
agners
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The key pretty much should be enough identifying information to derive the message from. I wonder if we should have made the message_template just a local, static field of the exception and not transmitted over the wire 🤔 .
Anyhow, it can come in handy to make small adjustments/improvements, and probably doesn't matter much that we transmit it over the wire.
|
@agners yea I agree. For some reason I thought I'd be reconstituting the message using the message template in the client library, that was why I added it. Then when doing the code here I realized that didn't make sense, I already had the message. Maybe I should drop the field from the client library for now? It already made the API but that doesn't mean it has to be added here if we don't need it. |
You ended up using the template from what I can tell no? The message template is only in the fixtures, which tries to reflect what Supervisor returns. Having the message template string on server side has the benefit that we can alter it. On the other hand it makes things more difficult for translations: Translators need to check Supervisor code to find the English original template 🤔 . |
agners
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now aligns with latest Supervisor change where we removed message_template again (see home-assistant/supervisor#6205).
LGTM!
41292e6 to
90c4f74
Compare
Proposed Changes
Add support for the addon availability API added in home-assistant/supervisor#6140