Use zeroconf attributes in elgato#58958
Conversation
|
Hey there @frenck, mind taking a look at this pull request as it has been labeled with an integration ( |
|
|
||
| async def async_step_zeroconf(self, discovery_info: dict[str, Any]) -> FlowResult: | ||
| async def async_step_zeroconf( | ||
| self, discovery_info: DiscoveryInfoType |
There was a problem hiding this comment.
Or maybe it would it be better to use zeroconf.HaServiceInfo ?
| self, discovery_info: DiscoveryInfoType | |
| self, discovery_info: zeroconf.HaServiceInfo |
There was a problem hiding this comment.
Actually, it’s unsafe to override a method with a more specific argument type, as it violates the Liskov substitution principle. For return types, it’s unsafe to override a method with a more general return type.
There was a problem hiding this comment.
I think we should adjust the upstream method typing for this in a second-round (once all integrations have been adjusted to ZeroconfServiceInfo).
There was a problem hiding this comment.
If we want to go ahead with this, why do it in a second pass? If @bdraco agrees, I can adjust async_step_zeroconf right away in config_entries.py. From my initial attempt, we just need to add a cast from ZeroconfServiceInfo to DiscoveryInfoType in the default implementation:
async def async_step_zeroconf(
self, discovery_info: ZeroconfServiceInfo
) -> data_entry_flow.FlowResult:
"""Handle a flow initialized by Zeroconf discovery."""
return await self.async_step_discovery(cast(DiscoveryInfoType, discovery_info))There was a problem hiding this comment.
I've just run some extra tests, and I think it might be easier to first change the result type to ZeroconfServiceInfo in config_entries.py, and then finish the "pass" on all the components.
If we do it now, then less code to change to satisfy mypy
I'll prepare a separate PR for this.
There was a problem hiding this comment.
Rebased following the merge of PR 59503
26c0543 to
4b9c2e3
Compare
4b9c2e3 to
0bfabce
Compare
Co-authored-by: epenet <epenet@users.noreply.github.com>
Proposed change
Use zeroconf attributes in elgato
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: