Skip to content

Fix push subscription serialization error#11437

Closed
Chris-V wants to merge 1 commit into
home-assistant:devfrom
Chris-V:fix_html5_json_bytes
Closed

Fix push subscription serialization error#11437
Chris-V wants to merge 1 commit into
home-assistant:devfrom
Chris-V:fix_html5_json_bytes

Conversation

@Chris-V
Copy link
Copy Markdown
Contributor

@Chris-V Chris-V commented Jan 3, 2018

Description:

PyWebPush updates the subscription reference held by home assistant with a byte encoded value. This causes errors the next time we try to save html5_push_registrations.conf. As a workaround, I do a deepcopy of the registration dictionary before giving it to PyWebPush.

I opened a PR in PyWebPush with the actual fix (web-push-libs/pywebpush#84) and it got accepted. So there are two other options here. Either:

  1. we wait until a new release is done; or
  2. we change the version of pywebpush to this exact commit.

Related issue (if applicable):
fixes #10751
fixes #11024

Pull request in home-assistant.github.io with documentation (if applicable): N/A

Example entry for configuration.yaml (if applicable):

N/A

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@Chris-V Chris-V force-pushed the fix_html5_json_bytes branch from 82aa093 to 2838071 Compare January 3, 2018 20:01
@Chris-V Chris-V changed the title Copy push subscriptions before passing them to pywebpush Fix push subscription serialization error Jan 5, 2018
@Chris-V Chris-V force-pushed the fix_html5_json_bytes branch from 2838071 to ea311c1 Compare January 5, 2018 23:03
@Chris-V Chris-V force-pushed the fix_html5_json_bytes branch from ea311c1 to 9febab6 Compare January 5, 2018 23:33
@balloob
Copy link
Copy Markdown
Member

balloob commented Jan 6, 2018

I just added a comment to your PR on pywebpush asking for a release. Let's give them a couple of days. Otherwise we'll merge this PR as it stands.

Could you add a link to your PR as a comment to the line with deepcopy. That way if we merge this PR as-is, we know where to check if we can remove it.

PyWebPush updates the subscription reference held by home assistant with
a byte encoded value. This causes errors the next time we try to save
html5_push_registrations.conf. As a workaround, I do a deepcopy of the
registration dictionary before giving it to PyWebPush.
@Chris-V
Copy link
Copy Markdown
Contributor Author

Chris-V commented Jan 6, 2018

Comment added!

@Chris-V
Copy link
Copy Markdown
Contributor Author

Chris-V commented Jan 7, 2018

pywebpush has been released. I opened a new PR with the upgrade: #11497

@Chris-V Chris-V closed this Jan 7, 2018
@home-assistant home-assistant locked and limited conversation to collaborators May 29, 2018
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.

Error in logs after enable push notifications Error saving html5_push_registrations.conf

4 participants