Common: Check_expired_locked_rules modernization. #127

[voetberg]

Changes:

• Change text-only queries to poll the data model (rucio.db.sqla.models)
• Push results to a remote (See documentation of probes for descriptions). Names: locked_expired_rules.(rse), locked_expired_rules.dids.(rse), locked_expired_rules.rules_for_dids.(rse)
  • included a check that still updates the probe even if the result is 0, to ensure the dashboards reading the probe always have data.

Sqla update uses pr #46 as a basis.

cc: @ericvaandering

[voetberg]

Thanks for the comments, those are fairly easy changes to make.

I suppose the confusion stems from my lack of understanding how atlas monitoring works - how are you picking up the results of these probes? If I knew that I think I could avoid making breaking changes in the future.

[dchristidis]

ATLAS uses Nagios to run its probes. Hence, the output is captured. Most of the time we don’t consult it, but this is one of the few probes where we do (so that our operators don’t have to identify themselves which rules to unlock).

[ericvaandering]

Sorry. Am I missing something? Don't lines 60 and 101 produce exactly the same output that 33 and 47 in the original do?

[ericvaandering]

The intent of that SQLAlchemy was definitely to reproduce the exact query. I would assume this was verified back some time ago (and one can have SQLAlchemy print the generated query). Is there something you saw?

[dchristidis]

Sorry. Am I missing something? Don't lines 60 and 101 produce exactly the same output that 33 and 47 in the original do?

The intent of that SQLAlchemy was definitely to reproduce the exact query. I would assume this was verified back some time ago (and one can have SQLAlchemy print the generated query). Is there something you saw?

The content of the commit has been altered since I made the initial review.

[voetberg]

Sorry. Am I missing something? Don't lines 60 and 101 produce exactly the same output that 33 and 47 in the original do?

The intent of that SQLAlchemy was definitely to reproduce the exact query. I would assume this was verified back some time ago (and one can have SQLAlchemy print the generated query). Is there something you saw?

The content of the commit has been altered since I made the initial review.

Yeah sorry about the confusion, original commit skipped the printing/summary dictionary step and just queried the count reported in the probe directly.

[ericvaandering]

Ahh. Good.

[dchristidis]

Side note: kindly allow me to resolve the conversations myself. It helps me to keep track of what has been addressed and what requires further work.

[voetberg]

Side note: kindly allow me to resolve the conversations myself. It helps me to keep track of what has been addressed and what requires further work.

Sounds good - let me know if there's anything else I can do to accommodate your preferred review style