Skip to content

Prevent accidental device reg override#17136

Merged
balloob merged 1 commit intodevfrom
do-not-accidental-override
Oct 8, 2018
Merged

Prevent accidental device reg override#17136
balloob merged 1 commit intodevfrom
do-not-accidental-override

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Oct 4, 2018

Description:

Prevent data loss in the data registry. Data could get lost if a second entity pointing at the same device would contain less information.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from a team as a code owner October 4, 2018 13:40
@ghost ghost assigned balloob Oct 4, 2018
@ghost ghost added the in progress label Oct 4, 2018
Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's look a bit easier if you use a schema they have extra=vol.REMOVE_EXTRA?

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Oct 5, 2018

The issue is that passing in None will use None as the value in the device registry. Not passing the parameter to the get_or_create function is the only way of not overriding it.

'sw_version',
'via_hub',
):
if key in device_info:
Copy link
Copy Markdown
Member

@OttoWinter OttoWinter Oct 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be if device_info.get(key) is not None:?

I think I've seen some device info implementations setting the values of keys to None to indicate absence. Or should that be changed per implementation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be changed in the implementation. If you set something to None, you indicate that it shouldn't be any value.

@balloob balloob added this to the 0.80 milestone Oct 8, 2018
@balloob balloob merged commit 4b7f855 into dev Oct 8, 2018
@balloob balloob deleted the do-not-accidental-override branch October 8, 2018 07:30
@ghost ghost removed the in progress label Oct 8, 2018
OttoWinter added a commit to OttoWinter/home-assistant that referenced this pull request Oct 8, 2018
balloob pushed a commit that referenced this pull request Oct 8, 2018
* Implement base for MQTT device registry integration

* Lint

* Lint

* Address comments

* Lint

* Lint

* Address comments

* Only add keys if specified

See #17136 (comment)
balloob added a commit that referenced this pull request Oct 8, 2018
@balloob balloob mentioned this pull request Oct 12, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants