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

Heidelberg: Revert pre-fill of streets and ignore timezone on Christmas tree collection parse #3502

Merged

Conversation

DerDreschner
Copy link
Contributor

@DerDreschner DerDreschner commented Jan 8, 2025

During inspection of the failed CI pipeline on my last pull request (#3497 / #3498), I recognized that using CONFIG_FLOW_TYPES with a public method call has unexpected side effects. Whenever the file is being imported (e.g., by running test_sources.py or update_docu_links.py), the Source.get_available_streets() method is being called in order to fill the CONFIG_FLOW_TYPES dictionary even without access to values of the dictionary at all. This leads to unnecessary and maybe excessive calls to the public API. It's visible in this pipeline run here.

First, I thought about answering calls from both test/debug scripts with an empty list using this workaround:

import inspect

[...]

    @staticmethod
    def get_available_streets() -> list[str]:
        if ("update_docu_links.py" or "test_sources.py") in inspect.stack()[-1].filename:
            return list()

But, honestly, I'm pretty sure there may be cases the file is being imported by HA/HACS I'm not aware of right now. And checking the stack-trace on runtime in production environments is pretty nasty. Therefore, I decided to revert this change entirely. In case the street entered by the user isn't correct, I call the API anyway and present the result as suggestions.

Additionally, I ignore the local timezone information now when parsing the Christmas tree collection date in order to avoid shifting it to another date unexpectedly.

Sorry for any inconvenience I caused!

@5ila5 5ila5 merged commit 98702f5 into mampfes:master Jan 10, 2025
2 checks passed
@5ila5
Copy link
Collaborator

5ila5 commented Jan 10, 2025

Thanks for your contribution

Custom pre-filled configuration flow are still very bad. They were an afterthought when creating the config_flow as it is now.
I want to look into this sometime, but I currently lack the time to do so.

mfortin pushed a commit to mfortin/hacs_waste_collection_schedule that referenced this pull request Jan 15, 2025
…as tree collection parse (mampfes#3502)

* Revert "feat(Heidelberg): Pre-fill available streets when using GUI for setup"

This reverts commit 5931e11.

* feat(Heidelberg): Make get_available_streets() function private as public visibility isn't needed anymore

* fix(Heidelberg): Ignore timezones when parsing christmas collecion date
Oberknecht pushed a commit to Oberknecht/hacs_waste_collection_schedule that referenced this pull request Feb 6, 2025
…as tree collection parse (mampfes#3502)

* Revert "feat(Heidelberg): Pre-fill available streets when using GUI for setup"

This reverts commit 5931e11.

* feat(Heidelberg): Make get_available_streets() function private as public visibility isn't needed anymore

* fix(Heidelberg): Ignore timezones when parsing christmas collecion date
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