-
Notifications
You must be signed in to change notification settings - Fork 111
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
Added state tracking and conditional event triggering for URL status … #212
base: main
Are you sure you want to change the base?
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
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Quality Gate passedIssues Measures |
@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. |
…changes
Added state tracking and conditional event triggering for URL status changes
client_error
to handle cases where an exception occurs.url_states
dictionary 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