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

add validation methods #41

Merged

Conversation

bj00rn
Copy link
Contributor

@bj00rn bj00rn commented Mar 19, 2024

Add methods for validation to support future implementation validation of parameters and api key in Home Assistant config flow in order to avoid creating broken sensors. To partially support home-assistant/core#106771

I had to add rate_limit and authenticate flags to _request method since /check endpoint does not provide rate limit headers in response and does not seem to exist behind authentication (at least not according to the docs)

@bj00rn bj00rn force-pushed the feat/add-validation branch from cb63ab2 to 22b235a Compare March 19, 2024 13:17
@bj00rn
Copy link
Contributor Author

bj00rn commented Mar 19, 2024

@bj00rn
Copy link
Contributor Author

bj00rn commented Mar 19, 2024

Seems that {api_key}/info endpoint does not provide code in response when 401 is returned for an invalid key, this causes an exception to be thrown in

super().__init__(f'{data["text"]} (error {data["code"]})')
when key is invalid. I've just tested with a nonsense key aaaaaaaaaaaaaaaa though som maybe results may vary depending if the key is well formatted or not.

image

Maybe it's worth into looking into cleaning up exception classes and defining arguments and not just passing data['message'] later on? Not sure what the pythonic way of doing this is though but passing arbitrary json to exception constructors should at least not throw another exception when keys are missing IMHO.

@bj00rn bj00rn force-pushed the feat/add-validation branch from 76230aa to 2f5fa5b Compare March 21, 2024 09:47
@bj00rn
Copy link
Contributor Author

bj00rn commented Mar 21, 2024

Depends on #43 for linting errors

* validate api key
* validate plane
@bj00rn bj00rn force-pushed the feat/add-validation branch from 2f5fa5b to 88af72f Compare April 1, 2024 09:44
@bj00rn
Copy link
Contributor Author

bj00rn commented Apr 1, 2024

@frenck @MartinHjelmare rebased now, tests are passing

forecast_solar/__init__.py Outdated Show resolved Hide resolved
forecast_solar/__init__.py Outdated Show resolved Hide resolved
@bj00rn bj00rn force-pushed the feat/add-validation branch from 88af72f to 4a52602 Compare April 2, 2024 11:11
@bj00rn bj00rn requested a review from klaasnicolaas April 2, 2024 15:41
@bj00rn
Copy link
Contributor Author

bj00rn commented Apr 7, 2024

@klaasnicolaas any chance of getting this merged soon? I Need this to resolve home-assistant/core#106771

forecast_solar/__init__.py Outdated Show resolved Hide resolved
forecast_solar/__init__.py Outdated Show resolved Hide resolved
@bj00rn bj00rn requested a review from klaasnicolaas April 8, 2024 09:51
@klaasnicolaas klaasnicolaas merged commit fa89261 into home-assistant-libs:master Apr 8, 2024
2 checks passed
@bj00rn
Copy link
Contributor Author

bj00rn commented Apr 8, 2024

@klaasnicolaas Thanks! Will you carve a new release with this fix?

@klaasnicolaas
Copy link
Contributor

@bj00rn Yes, release v3.1.0 is now available.

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