-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Validators for buffering key extraction #1255
Conversation
validators | ||
end | ||
|
||
class PlaceholderValidator |
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.
I think separating PlaceholderValidator into TagPlaceholderValidator, TimePlaceholderValidator, KeysPlaceholderValidator is better.
It removes time?
, tag?
and keys?
helpers and reduces method call overhead in validate!
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.
No, it doesn't work. We should provide the possibility to plugin authors, for selecting which validator should be called or not.
For example, table
might not have timestamp placeholders even when time
chunk key is configured for any databases which can handle automatically generated time partitions. In such cases, the validator for time can be skipped by plugin authors (by using validator.time?
)
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.
I see.
def get_placeholders_tag(str) | ||
# [["tag"],["tag[0]"]] | ||
parts = [] | ||
str.scan(/\$\{(tag(?:\[\d+\])?)\}/).map(&:first).each do |ph| |
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.
Use constant value to share regexp.
extract_placeholders
also uses same regexp.
I added a commit for tag placeholder. |
c3f4260
to
9eb470e
Compare
I rebased on current master and added a commit for regexp. |
It seems good. |
This change is mainly to make plugin authors possible to check configuration values are valid or not in the viewpoint of placeholders, like
${tag}
,%Y-%m-%d
or${any_key}
s.Users can write any placeholders in their configuration parameters if plugin author provides a feature to extract it with buffer chunk keys. But buffer chunk keys and placeholders used in these values MUST be consistent. This consistency is very easy to be broken.
Newly introduced methods provides features to validate these parameters and buffering configurations.
This change is based on #1254.