-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(inputs.postgresql_extensible): introduce MaxVersion #13620
Conversation
I can't run tests on my M2 mac, something about docker timing out |
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 the PR @redbaron! Just some minor comments...
@@ -37,7 +37,9 @@ type Postgresql struct { | |||
type query []struct { | |||
Sqlquery string | |||
Script string | |||
Version int | |||
Version int `deprecated:"1.28.0;use minVersion to specify minimal DB version this query supports"` |
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.
Version int `deprecated:"1.28.0;use minVersion to specify minimal DB version this query supports"` | |
Version int `deprecated:"1.28.0;use 'min_version' instead"` |
MinVersion int | ||
MaxVersion int |
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.
Please add TOML tags for those options!
# minversion int | ||
# maxversion int |
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.
By default TOML will name them min_version
and max_version
if I'm not mistaken...
How are you running tests? Are you running |
Thing is, this plugin tests require PostgreSQL container
I am running from VSCode, which internally close to |
Integration test failure seems to be unrelated |
Correct vscode will run go test without the |
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 👍 This pull request doesn't change the Telegraf binary size 📦 Click here to get additional PR build artifactsArtifact URLs |
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.
Thank you!
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.
Looks good to me. Thanks for the nice feature @redbaron!
Required for all PRs
resolves #13568
Rename
version
tominversion
and introducemaxversion
to limit DB version query should run on.