feat: expose NWS geocode (UGC/SAME) on alert attributes#142
Conversation
Adds a Geocode dict to each alert containing the UGC and SAME code lists from the NWS properties.geocode object. Uses .get fallbacks so a missing geocode key cannot raise mid-update and take the whole refresh down.
|
Something similar was requested recently but I rejected the SAME Code but added the NWS Event code. And I didn't even touch on the fact that attributes being limited to 16k is a potential issue. I've rejected other requests due to this but the NWS event codes were a small addition and added better functionality. If I keep adding everything then the data becomes unwieldy at some point. It's almost there now :) |
|
Thanks for the context on #112, makes sense! I'll drop SAME from this PR. UGC zones might still be worth keeping though since they're unambiguous identifiers. I can flatten this further into a On the 16KB limit, I have a couple of ideas I wanted to run by you:
Happy to prototype either direction in a follow-up if you're interested. |
|
I really don't think that there are many users using multiple zones. And the alert info for any users that do would most likely be just duplicates of the other zones. For example I use an id for my county and my alert zone (INC033 & INZ009) but both of those pretty much always have the same alerts (as you would expect). Splitting them wouldn't make much sense from a data usefulness standpoint and if it did split them then it would be littering the DB with redundant data all the time. The only time I see splitting it up into a sensor per id is if a user is monitoring two different locations (either static or dynamic). But even then if the user simply uses two different integration instances using those different id's then the integration already creates a different sensor for each instance. So it's already doing moistly what you are suggesting by default. The only time it doesn't is if the user adds more than one id to the same instance. But again that's up to the user. I actually had a request to split the sensors up into different entities for each alert that came in. But I rejected that just due to the complexity of being necessary to monitor for a new sensor creation on alert issue and then the old sensor disappearing when the alert expired. And I'm still having a hard time seeing how adding the geocode data helps a user monitor for alerts. It seems to me that all the geocode data tells you is where all the id's the alert that was issued is active. But if the user has configured their integration to use an id that was in the geocode list then they would already have gotten the alert sent to them. Maybe I'm missing something on what value the extra data adds. Maybe I might think about a config option that allows users to pick which field they allow but my concern (as always) is the effort needed in maintaining that more complex integration. Even now this has gotten beyond me being able to maintain this to a point. I can't guarantee that I'll always be able to rely on others to fix things that break in the future. All that to say he integration works well as it is so I'm pretty hesitant to add the extra info. |
|
Thanks again for the in depth feedback. You've been incredibly patient and helpful with me and I really appreciate it. I do understand why someone would ask for entity-per-alert (seems like the natural demarcation to avoid the 16k limit altogether), but the thought of trying to maintain/support that in weather_alerts_card horrifies me. entity-per-zone at least maps cleanly to "areas" and wouldn't require additional dynamic entity management. My primary configuration is exactly the outlier you mentioned, where I'm monitoring multiple overlapping and non-overlapping zones with a single entity. From what you're describing, it seems like the ideal approach for this situation should be to use multiple entities instead of the multi-zone entity. I'm conflicted because users already choose their zones explicitly, and the same result is achievable with multiple entities, but that duplicates DB entries and API calls. To me, the standout feature of the multi-zone entity is that it results in a single API call to retrieve updates for multiple zones. It encourages being a good API citizen and feels like a nice consideration. It would make me very happy if I could share a single API call for multiple entities - I do want to avoid overengineering (and additional maintenance burden), so let me know if that's something you'd consider. Related: while looking at the batch API calls, I noticed the NWS API guidance to mitigate potential abuse. One of the key points here is that their automated security enforces on the The official NWS integration uses this format: Would you be open to a PR for the User-Agent change? Happy to keep the shared-call idea on the back burner. Feel free to close this one out or I can rework it. Thanks again for the review! |
|
I never considered the idea of someone abusing the API call getting everyone blocked scenario. I think it's highly unlikely since the integration has been around for years now and it hasn't happened yet. But who's to say that as it potentially gets more popular that it won't happen. I would definitely be open to changing the user-agent header so I don't get my instance blocked by someone else being ridiculous. And honestly I think I might actually be the one being ridiculous since I have two or three instances of HA running sometimes with 2 or 3 instances of the integration also running on each HA instance for test purposes. So I myself could have up to 10 calls happening at some point in time. But likely most users should typically have 1 running integration instance or a couple if they use the dynamic method. as far as using one API call for every instance of the integration I'm not sure how that would work. Wouldn't there need to be some interaction between every instance of the integration on a users system so that all of the API calls collect every possible zone or lat/lon that might be configured and bundle them all up into one call? then you would need to store that returned data somewhere so the each integration instance can retrieve and act on it. And TBH I'm not even sure that the API is set up to make a zone id call and a single point lat/lon call at the same time. So you at least would need 2 calls to the API in that situation anyway. Is it really worth the extra complexity for minimal impact on the API? |
|
I agree on all counts. I suspect I would be another ridiculous user. I manage about as many installations, and trialed one briefly with 20 instances when I noticed the calls. Surely the vast majority of users only use a single instance! The shared call would require a data fetching layer to feed the individual coordinator instances, and that definitely involves meaningful overhead for a small gain. You're also right on a separate fetcher required for each lat/long and zones, I hadn't considered that. I'll close this out and submit a PR for the User-Agent fix. |
2 participants
Changes
Adds a Geocode dict to each alert containing the UGC code list from the NWS properties.geocode object.
Motivation
This enables zone sorting/filtering in frontend components.
Why Draft?
HA Entity Attribute limit is 16KB, this would count towards that and potentially break existing setups hitting that limit (like #95)
Keeping this one as a Draft until mitigation approach is defined.