search

ansible / eda-server

Event Driven Ansible for AAP
Apache License 2.0
64 stars 41 forks source link

WIP: refactor: reduce code duplication in request processing #892

Closed jshimkus-rh closed 1 month ago

jshimkus-rh commented 4 months ago

This provides the agreed upon usage of PENDING w/ status text containing the specific message:

The workers available to process request type {request_type} for
{process_parent_type} {process_parent_id} are failing liveness checks.
There may be an issue with the node; please contact the administrator.

as distinguishing a request that cannot be run due to complete lack of workers.

The message text above is slightly modified from that documented in AAP-23378 for consistency with other log messages from the processing code.

Orchestrator log messages were updated due to previous change that specifically set the request type if none was specified.

ttuffin commented 4 months ago

To me that message is contradictory - it implies there are workers available, when in fact they are not responding to liveliness checks, which in essence means they are unavailable.

jshimkus-rh commented 3 months ago

To me that message is contradictory - it implies there are workers available, when in fact they are not responding to liveliness checks, which in essence means they are unavailable.

@ttuffin Updated per our slack convo.

jshimkus-rh commented 3 months ago

All outstanding issues and critiques have been addressed.

Alex-Izquierdo commented 3 months ago

HI @jshimkus-rh I'm finally covered this with https://github.com/ansible/eda-server/pull/894

jshimkus-rh commented 3 months ago

HI @jshimkus-rh I'm finally covered this with #894

True. However, I saw that as a stopgap to fixing the regression from #880.

Alex-Izquierdo commented 3 months ago

I'm addressing some other issues there as well. Also, I'm not sure, but I had there to update some tests, which makes sense since we are changing the behavior. I'm receiving some "pressure" from mates because right now the devel builds are mostly in an useless state, I'm trying to finish it as soon as possible to unblock it and continue working on other issues. I would like to take advantage of the situation to extend the coverage of the integration tests, but due to the rush for the fix I think we can do it in later PR's.

I suggest to continue working there, because I'm already tested it in different deployments and try to have it ready for merge soon. Hopefully we can have a stable version and we continue working from there with the rest of issues.

jshimkus-rh commented 3 months ago

Converted to draft. To be revisited post next release.