-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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 Azure data explorer #119089
Fix Azure data explorer #119089
Conversation
Refactor the initialization of the Azure Data Explorer client to separate the creation of the ingest and query clients. Thisremoves the bug where the wrong URL was used when either testing for connectivity or inserting data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the merge conflict and describe your changes in the PR description
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Ok, I have two errors in my dev setup 1. The filter as you describe and when I comment out the filter, no data is delivered to ADX but with no errors. Both works perfectly on my HACS setup with more or less same code
https://github.com/kaareseras/adx_hacs
…________________________________
Fra: i-am-shodan ***@***.***>
Sendt: Friday, June 7, 2024 5:29:36 PM
Til: home-assistant/core ***@***.***>
Cc: Kaare Rasmussen ***@***.***>; Author ***@***.***>
Emne: Re: [home-assistant/core] Adx bug (PR #119089)
image.png (view on web)<https://github.com/home-assistant/core/assets/6901273/7c925cb3-4dc3-44bc-b6d0-e6366c1d5e60>
Looks like I'm getting a filtering error, I have no filters
—
Reply to this email directly, view it on GitHub<#119089 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADTSC63E4QEI6VIJFW7TI5DZGHGWBAVCNFSM6AAAAABI6ZSCISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGA3TAOJQGU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No clue if this change is the right one, but have a review on the strings and some comments
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as abuse.
This comment was marked as abuse.
if DOMAIN not in yaml_config: | ||
return True | ||
hass.data[DOMAIN][DATA_FILTER] = yaml_config[DOMAIN].pop(CONF_FILTER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if DOMAIN not in yaml_config: | |
return True | |
hass.data[DOMAIN][DATA_FILTER] = yaml_config[DOMAIN].pop(CONF_FILTER) | |
if DOMAIN in yaml_config: | |
hass.data[DOMAIN][DATA_FILTER] = yaml_config[DOMAIN].pop(CONF_FILTER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not sure why, but this change will cause many test to fail
FAILED tests/components/azure_data_explorer/test_init.py::test_connection[Exception] - AssertionError: assert <ConfigEntryState.NOT_LOADED: 'not_loaded'> == <ConfigEntryState.SETUP_ERROR: 'setup_error'>
- where <ConfigEntryState.NOT_LOADED: 'not_loaded'> = .state
- and <ConfigEntryState.SETUP_ERROR: 'setup_error'> = ConfigEntryState.SETUP_ERROR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are running successfully on my machine with the above change. Can you please recheck?
ha-venv@edenhaus ➜ /workspaces/core (pr/kaareseras/119089) $ pytest tests/components/azure_data_explorer
Test session starts (platform: linux, Python 3.12.3, pytest 8.2.0, pytest-sugar 1.0.0)
rootdir: /workspaces/core
configfile: pyproject.toml
plugins: mock-3.14.0, rerunfailures-14.0, typeguard-4.2.1, aiohttp-1.0.5, picked-0.5.0, requests-mock-1.12.1, respx-0.21.1, cov-5.0.0, sugar-1.0.0, timeout-2.3.1, socket-0.7.0, unordered-0.6.0, github-actions-annotate-failures-0.2.0, syrupy-4.6.1, xdist-3.6.1, asyncio-0.23.6, pytest_freezer-0.4.8, anyio-4.3.0
asyncio: mode=Mode.AUTO
collected 19 items
tests/components/azure_data_explorer/test_config_flow.py ✓✓✓ 16% █▋
tests/components/azure_data_explorer/test_init.py ✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓ 100% ██████████
Results (1.12s):
19 passed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing :-)
Results (3.27s):
11 passed
8 failed
- tests/components/azure_data_explorer/test_config_flow.py:15 test_config_flow
- tests/components/azure_data_explorer/test_init.py:153 test_filter[allowlist]
- tests/components/azure_data_explorer/test_init.py:153 test_filter[denylist]
- tests/components/azure_data_explorer/test_init.py:153 test_filter[filtered_allowlist]
- tests/components/azure_data_explorer/test_init.py:153 test_filter[filtered_denylist]
- tests/components/azure_data_explorer/test_init.py:268 test_connection[KustoServiceError]
- tests/components/azure_data_explorer/test_init.py:268 test_connection[KustoAuthenticationError]
- tests/components/azure_data_explorer/test_init.py:268 test_connection[Exception]
and when debugging the code, I get this error:
2024-06-10 16:45:40.022 ERROR (MainThread) [homeassistant.setup] Error during setup of component azure_data_explorer
Traceback (most recent call last):
File "/workspaces/core/homeassistant/setup.py", line 404, in _async_setup_component
result = await task
^^^^^^^^^^
File "/workspaces/core/homeassistant/components/azure_data_explorer/init.py", line 69, in async_setup
hass.data[DOMAIN][DATA_FILTER] = yaml_config[DOMAIN].pop(CONF_FILTER)
~~~~~~~~~~~^^^^^^^^
KeyError: 'azure_data_explorer'
And the integration does not load
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something wrong with your dev environment. Can you please check that your are using the latest version of all requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kaareseras 👍
Co-authored-by: Robert Resch <[email protected]>
Breaking change
Proposed change
Change the strings file to give a better explanation of what to insert
Add back the difference between the -ingest and non -ingest URI
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: