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

Improve placeholder #115

Merged
merged 4 commits into from
Mar 13, 2017
Merged

Improve placeholder #115

merged 4 commits into from
Mar 13, 2017

Conversation

joker1007
Copy link
Collaborator

Enable to use placeholder for project, dataset, fetch_schema_table.

Because of it, fetch_schema timing is delayed.

@joker1007 joker1007 force-pushed the improve-placeholder branch from dda9f35 to e4facd5 Compare March 10, 2017 00:53
@tagomoris
Copy link
Contributor

It's better to check whether the values from configuration parameters use invalid placeholders or not by calling placeholder_validate!(name, str) in #configure.
That method raises configuration error if ${tag} is used in str but configured buffer chunk key doesn't have tag (it also checks about other keys).

@joker1007
Copy link
Collaborator Author

@tagomoris Thanks!

I wanted to use placeholder_validate!, but I feel that current placeholder_validate! is not easy to use for me.
Because it is seemed that placeholder_validate! requires placeholder syntax in parameter.

For example in config,

<buffer time>
  timekey 1d
</buffer>

project yourproject_id
dataset yourdataset_id
table target_table_%Y%m%d # I want to use placeholder only table param in this config.

in this case, placeholder_validate! regards project and dataset as invalid.
The behavior is not suitable for this plugin.

Of course, I can guard placeholder_validate! if it is not required (ex. use regexp matching).
Is this intended usage?

@tagomoris
Copy link
Contributor

@joker1007 It's intended - Once I tried to design validator API to validate combination of 2 or more configuration parameter values and configured buffer chunk keys, but I found that it's too difficult to both implement and use it.

For this case, we can validate the whole configuration and chunk keys by the code like below:

placeholder_validate!(:project_and_database_and_table, "#{@project}/#{@database}/#{@table}")

It shows error message a bit difficult to understand, but it's much better than occurring problems after Fluentd starts.

@joker1007
Copy link
Collaborator Author

I understand API design. Thanks!!
the snippet is looks good to me.
I will add placeholder_validate!.

@joker1007 joker1007 force-pushed the improve-placeholder branch from 6935316 to e0153c8 Compare March 13, 2017 15:42
@joker1007 joker1007 merged commit 326fa6a into master Mar 13, 2017
@joker1007 joker1007 deleted the improve-placeholder branch March 13, 2017 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants