Skip to content
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

Allow back passing a list of Query objects #7979

Closed
wants to merge 1 commit into from

Conversation

florimondmanca
Copy link
Contributor

What does this PR do?

This is a cleanup for #7750

Motivation

#7750 changed QueryManager so that queries=... is expected to be a list of dict, rather than a list of Query.

The motivation was that since queries are statically declared in a queries.py module and referenced from there, we wanted to avoid passing those references directly, and so we unwrapped everything from Query to pass queries=[<dict>, ...].

Yet by doing so we broke backwards compatibility (the PR does have a changelog/Changed label). Here's why:

  • On < 7.24.0, checks must be written as QueryManager(queries=[Query(<dict>), ...]).
  • On 7.24.0+, checks must be written as QueryManager(queries=[<dict>, ...]) (and it internally converts them to Query).
  • And those two are incompatible.

This means users won't be able to upgrade database integrations to versions released after #7750 without also upgrading their Agent to 7.24.0 (which hasn't been released yet, and won't be for a few weeks still, although RCs are available).

For my particular use case (VoltDB), it means I'm not able to write a voltdb that can both work on older Agents (eg 7.22.0, or whichever Agent the customer is using right now) and master. Yet I'd like to avoid requiring the prospect to update their Agents to use an RC — not a very sexy requirement for a sales pitch. :-)

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@florimondmanca florimondmanca mentioned this pull request Nov 9, 2020
5 tasks
@florimondmanca
Copy link
Contributor Author

Closing, we don't expect custom checks to be using this anyway, and for my use case (need to support an Agent < 7.24.0 until the final release is out, while making tests pass on master) I'll just try/except my way through things.

@florimondmanca florimondmanca deleted the fm/db-query-obj branch November 10, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant