Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix wallbox jwt issue #79948

Merged
merged 11 commits into from
Oct 14, 2022
Merged

Fix wallbox jwt issue #79948

merged 11 commits into from
Oct 14, 2022

Conversation

hesselonline
Copy link
Contributor

@hesselonline hesselonline commented Oct 9, 2022 •
edited by MartinHjelmare
Loading

Proposed change

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@hesselonline
Copy link
Contributor Author

@cdce8p or @epenet does either of you have time for a review? Basically I'm trying to implement a fix for an issue which currently prevents the wallbox integration from working. Root cause of this issue is a JWT token of the api that expires (earlier than expected). The fix I'm implementing here is forcing the token to be empty so it is always being refreshed. I consider this a temp fix; everybody is now suffering from this issue and this solution works. However, I'm trying to find a more elegant solution in which the token is refreshed less often.

@hesselonline hesselonline requested a review from balloob October 10, 2022 17:25
@MartinHjelmare MartinHjelmare changed the title Wallbox fix jwt issue Fix wallbox jwt issue Oct 10, 2022
@hesselonline
Copy link
Contributor Author

@balloob small bump, I really like this PR to be merged to fix the issue. I think that the change now is quite minor (but the issue quite significant)

@hesselonline
Copy link
Contributor Author

@cdce8p or @epenet maybe one of you has time for a review? Hate to be pushy, but this is a fix for an issue which currently blocks the use of the wallbox integration.

@MartinHjelmare
Copy link
Member

Please update the PR description according to the current PR changes.

Please add a link to a changelog or GitHub commit compare view for the version bump in the PR description. Thanks!

@hesselonline
Copy link
Contributor Author

Please update the PR description according to the current PR changes.

Please add a link to a changelog or GitHub commit compare view for the version bump in the PR description. Thanks!

Done

@hesselonline hesselonline requested review from MartinHjelmare and removed request for balloob October 14, 2022 08:11
@MartinHjelmare
Copy link
Member

I added a link to the release.

@hesselonline @MartinHjelmare
Update homeassistant/components/wallbox/__init__.py
d03014d 
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
MartinHjelmare
MartinHjelmare approved these changes Oct 14, 2022
View reviewed changes
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Thanks!

@MartinHjelmare MartinHjelmare added this to the 2022.10.4 milestone Oct 14, 2022
@MartinHjelmare MartinHjelmare self-assigned this Oct 14, 2022
@MartinHjelmare MartinHjelmare removed their assignment Oct 14, 2022
@MartinHjelmare MartinHjelmare merged commit a534c13 into home-assistant:dev Oct 14, 2022
balloob pushed a commit that referenced this pull request Oct 14, 2022
@hesselonline @MartinHjelmare @balloob
Fix wallbox jwt issue (#79948)
3311a09 
* Bump Wallbox package

* remove debug message

* Force update of auth token by emptying it first

* Force token refresh by emptying token
Improve exception handling

* include tests

* Update __init__.py

* Removed the clearing ot jwt token, issue is fixed by upstream fix in wallbox package.

* Catch connectionerror

* Update homeassistant/components/wallbox/__init__.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Run black

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@balloob balloob mentioned this pull request Oct 14, 2022
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@balloob balloob balloob left review comments

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees
No one assigned
Labels
bugfix by-code-owner cherry-picked cla-signed Hacktoberfest has-tests integration: wallbox small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
2022.10.4
Development

Successfully merging this pull request may close these issues.

Intermittent Wallbox connection errors
4 participants
@hesselonline @MartinHjelmare @balloob @homeassistant