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.

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

Retry deprecation warning fix #9

Merged
merged 13 commits into from
Nov 8, 2024

Conversation

ejpenney
Copy link
Contributor

Retries #7

Sorry about that issue before.

@ejpenney
Copy link
Contributor Author

Still test issues, I'm gonna take another stab at this. Give me a few minutes...

@ejpenney
Copy link
Contributor Author

So far this adds the breaking change: Increased requirement to 2022.7.0 (and therefore Python>=3.9)

But these dependency updates have introduced other issues, I'm still poking at this.

@ejpenney ejpenney marked this pull request as draft April 25, 2024 15:48
@ejpenney
Copy link
Contributor Author

Stumped at this point, I'll take another look at it later, but this UnknownHandler error makes no sense. Looks to me like the domains are properly configured and everything, I'm clearly missing something.

@ejpenney
Copy link
Contributor Author

Upgrading to newer versions of Python/Home Assistant make a lot of these errors go away, but new errors appear. I suspect a problem with the tests, so much so that I see import_blueprint even removed tests from upstream.

I've tested this change as-is locally, and all the flows appear to work, but the tests are broken as-is.

@ejpenney ejpenney force-pushed the master branch 2 times, most recently from f3750dc to 2bffdd0 Compare April 25, 2024 18:31
@InTheDaylight14
Copy link
Owner

Here are some changes to the tests from another custom component: https://github.com/alandtse/tesla/pull/892/files

This integration is where I was originally inspired to make this custom integration. I'll be honest, a lot of this is outside my wheelhouse. I only dabble in this space and I'm no professional coder.

@ejpenney
Copy link
Contributor Author

I've narrowed it down to a problem with the node the tests are running on here in GitHub, I have an environment running Python 3.8, and all tests pass as the code is now. It only fails in GitHub.

image

@ejpenney ejpenney force-pushed the master branch 2 times, most recently from 65e3749 to 743ae91 Compare April 26, 2024 16:05
@ejpenney
Copy link
Contributor Author

Thanks, but I'm not seeing anything in that PR that seems particularly helpful. They've pinned some versions, but pip is more-or-less pulling those same versions here.

I see the reverted PR from the other day also has this UnknownHandler failure also. It looks like something changed upstream between October and now and is causing this failure. I have no idea what the issue is, but I suspect something related to a GitHub Action or the ubuntu-latest nodes. It could be a pip dependency needs to be pinned, but that seems increasingly unlikely the more I poke at it.

I've tried the latest GHA, as well as pinning to python-3.8.19 (the action is picking 3.8.18), but the first makes no difference, and the latter fails (apparently the GitHub node can't find any later versions?!).

I've refactored the change to be extremely simple, based on the changes to upstream, we're not queueing the task anymore at all. Also updated some HACS metadata for this repository while I was there, because... why not? Only thing I can say for sure is the test failures on this PR are not related to the changes in this PR. I tested this again in my development environment and everything appears to work.

I really dislike that I can't get the test working in GitHub, but I'm stumped (again).

@InTheDaylight14
Copy link
Owner

Got it, thanks for investigating. With a year before the functionality being depreciated I'm okay to sit on this PR for a little while.

I've always found the GitHub test environment to be finicky, given the tests work locally I might be okay with removing the GitHub tests. I'll keep thinking on it.

@InTheDaylight14 InTheDaylight14 marked this pull request as ready for review November 8, 2024 02:34
@InTheDaylight14 InTheDaylight14 merged commit 26ceaec into InTheDaylight14:master Nov 8, 2024
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants