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

fix(config): return error on order set as string #12880

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

powersj
Copy link
Contributor

@powersj powersj commented Mar 15, 2023

Order should be a number, not a string, so instead of silently failing we should call the user to action.

fixes: #12879

Order should be a number, not a string, so instead of silently failing
we should call the user to action.

fixes: influxdata#12879
@powersj powersj added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Mar 15, 2023
@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Mar 15, 2023
@powersj
Copy link
Contributor Author

powersj commented Mar 15, 2023

Would you prefer this attempt to parse a string to an int first? Or tell and expect the user to use an int?

Currently we are silently ignoring the value, and so a user using processors with a string order is not getting the order they want.

Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we have a test-case for this so it will always work?

For me it is file like this; erroring out as the example config shows integer values and docs mention it starts at 1.

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for fixing this @powersj!

@srebhan srebhan merged commit f1da63d into influxdata:master Mar 21, 2023
srebhan pushed a commit that referenced this pull request Apr 3, 2023
@srebhan srebhan added this to the v1.26.1 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defining Processor order as a string is silently ignored. Undefined Processor order runs in reverse.
3 participants