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

fix: public url missing #2676

Closed
wants to merge 3 commits into from

Conversation

danielbrunt57
Copy link
Collaborator

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Nov 12, 2024

I don't understand this error:

E ImportError: cannot import name 'in_progress_instances' from partially initialized module 'custom_components.alexa_media.config_flow' (most likely due to a circular import)

I've not touched changed that part since the def name error was fixed and a filter added to include only flows where entry["handler"] == DOMAIN to ensure accurate identification and management of in-progress flows for the Alexa Media integration (ref #2541).

I don't understand the possible circular reference or how to rectify it.

@alandtse
Copy link
Owner

Yah no idea. I'd assume the test handler has a circular import but no idea.

@alandtse
Copy link
Owner

Wow, that's a lot of changes to the config flow. Does it have to be that much?

@danielbrunt57
Copy link
Collaborator Author

danielbrunt57 commented Nov 15, 2024

Maybe I went overboard trying to address possible complications from yaml setup?
Personally, I'd rather deprecate legacy yaml setup with no automatic import.
I think I can code it like Deprecate YAML configuration #3163. What version should I reference for such a breaking change?

"YAML configuration of Alexa Media Player is deprecated and will be removed in version x.x.x and there will be no automatic import of this.\nPlease remove it from your configuration, restart Home Assistant and use the UI to configure it instead."

In view of this, do not merge but close it instead and I'll do a new PR that reverts the changes in 4.13.8, unless you can just delete v4.13.8 and revert to 4.13.7 as the current version?

Looking for guidance on the best way to proceed.
Perhaps a new PR reverting the changes in 4.13.8 with the YAML deprecation warning as of v4.14.0 added?
Note: I never encountered an issue with public_url while using UI setup.

@alandtse
Copy link
Owner

We can't just delete releases as it will confuse anyone who downloaded it.

@alandtse
Copy link
Owner

Ok, so the issue I have here is this PR says it's fixing public urls but appears to do much more. Please make sure you do things atomicly so that we can easily revert things if need be. If you want to just do a revert of the last pr, you can submit a PR for that. Then we can do whatever you're doing to the import code. Again, the import code really shouldn't be used by anyone as we haven't really supported yaml for like 3 years.

@danielbrunt57
Copy link
Collaborator Author

Okay, I'll do a PR with code from 4.13.7 to undo 4.13.8.

As for legacy yaml, I think it best to deprecate it and I'll not worry about trying to fix errors related to it.
I'll submit a new PR which initiates the warnings. What version should I state when it will no longer work? 4.13.x? 4.14.0?

@danielbrunt57 danielbrunt57 deleted the fix_public_url branch November 16, 2024 05:50
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