Added state tracking and conditional event triggering for URL status … #212
Conversation
…changes Added state tracking and conditional event triggering for URL status changes - Added initialization for `client_error` to handle cases where an exception occurs. - Introduced `url_states` dictionary to track the last known status of each URL. - Modified the main loop to include state checking and trigger events only if the URL status has changed, preventing duplicate rule triggers when the URL status transitions from down to up. These changes ensure more accurate state management and reduce unnecessary event triggers.
…L status has changed Improve URL status polling by adding state tracking and conditional event triggering - Initialize client_error to handle cases where an exception occurs - Add url_states dictionary to track the last known status of each URL - Modify event triggering to occur only if the URL status has changed
Alex-Izquierdo
left a comment
Alex-Izquierdo
left a comment
There was a problem hiding this comment.
Thanks for your contribution @tarunchy
| async with session.get(url, ssl=verify_ssl) as resp: | ||
| status = "up" if resp.status == OK else "down" | ||
| status_code = resp.status | ||
| except aiohttp.ClientError as exc: |
There was a problem hiding this comment.
I think to set the status to "down" is not correct, since the ClientError can be raised by a variety of reasons that doesn't necessarily mean the url is down, like for example networking issues in the client side. This would be something that the plugin is already doing wrong in the upper try/except block.
Instead, I suggest a new option with "false" as default named for example "send_error_events" and send the ClientError as event if its set.
thoughts? @bzwei
| status = "down" | ||
| status_code = None | ||
| client_error = str(exc) | ||
| # Only trigger event if the status has changed |
There was a problem hiding this comment.
I'm not sure if that logic should be handled by the plugin and it would change the existing behavior so it could break existing rulebooks.
|
|
@tarunchy Any updates? If is missing current release, it might be delayed notably. |
|
Apologies for the delay in response. We received a workaround from Red Hat support that appears to be working with our current code. The workaround involved increasing the delay in the url_check plugin to 360. It seems that this adjustment resolved the issue. The code I provided should work as well, but this alternative workaround also seems effective. Please let me know if any further action is required from my side, and I’m happy to assist. |
|
When @Alex-Izquierdo is back from PTO, he will know what to do with PR. Thanks for the quick answer. |
|
Hi, I commented some concerns about the current implementation that I would like to solve prior to merge it. If you are happy with the workaround and close this PR, that's up to you and would be fine to me. |
|
Hello @tarunchy , #432 fixed this issue by making sure that all the URLs are checked, which was a good part of what this PR does. The error reporting done has not changed: in case an URL is not reachable, the "status" field is "down" and the exception of the error (not only related to HTTP) is in the "error_msg" field. As @Alex-Izquierdo mentioned, changing that in any way (e.g. by providing the HTTP status code) is a separate change, and this PR may be repurposed for that. |
4 participants
…changes
Added state tracking and conditional event triggering for URL status changes
client_errorto handle cases where an exception occurs.url_statesdictionary to track the last known status of each URL.These changes ensure more accurate state management and reduce unnecessary event triggers.
I work in large health insurance company who use this tool and we are facing issue. We found this could be the root cause we will also raise support ticket through format channel