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

[WIP] v0.14 plugin APIs #562

Closed
wants to merge 111 commits into from
Closed

[WIP] v0.14 plugin APIs #562

wants to merge 111 commits into from

Conversation

tagomoris
Copy link
Member

Plugins and PluginSupport APIs for v0.14 plugin system.

Related: #309 #287

@tagomoris tagomoris force-pushed the v0.14 branch 3 times, most recently from 4f62aa2 to edf37b2 Compare May 29, 2015 11:17
@repeatedly repeatedly added feature request *Deprecated Label* Use enhancement label in general v0.14 labels Jul 7, 2015
@@ -0,0 +1,10 @@
<source>
Copy link
Member

Choose a reason for hiding this comment

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

in_status will be revmoed. so no need this configuration.

@repeatedly
Copy link
Member

plugin_support should be plugin/support?
I'm not sure Ruby's manner.

@nurse What do you think?


@queue_length_limit = @buffer_config.queue_length_limit
if @queue_length_limit && @total_bytes_limit > @chunk_bytes_limit * @queue_length_limit
@total_bytes_limit = @chunk_bytes_limit * @queue_length_limit
Copy link
Member

Choose a reason for hiding this comment

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

log.warn is needed.

@nurse
Copy link
Contributor

nurse commented Jul 24, 2015

plugin_support should be plugin/support?
I'm not sure Ruby's manner.

Ruby doesn't have related manner, but on Rails "plugin_helper" or "helpers" seem related words.

end
end

def metadata(timekey: nil, tag: nil, key_value_pairs={})
Copy link
Contributor

Choose a reason for hiding this comment

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

typo? key_value_pairs should be keyword argument.

-def metadata(timekey: nil, tag: nil, key_value_pairs={})
+def metadata(timekey: nil, tag: nil, key_value_pairs: {})

 * add `paths` to add optional load_path for each registory instance
 * add `require`s to load plugin classes in each tests
@tagomoris
Copy link
Member Author

Now I'm rebasing on current master.

@tagomoris
Copy link
Member Author

TODO:

  • syntax conversion w/ recent ruby style
  • add raise NotImplementedError in methods to be overridden in sub classes (currently commented out)
  • pass plugin logger to buffer plugins from buffered output plugins
  • add missing descriptions for plugin configurations

* and separate each output plugin classes into files
* Fluent::** are preserved for compatibility of traditional plugins
 * to reduce duplicated implementation in Input and Output
…tions

instead of Actor of v11
Plugins should include using PluginSupport Mixin individually.
* and mark old TestDriver as obsolete
 * not to specify default run_timeout & expected_emit_length
 * to miss to revise obsolete method/variable names
* DEFAULT_TYPE 'json'
* storage.synchronize should be callable more than once from the same thread
  * to create transactional get-put operation
  * by using Monitor
* non-saved data storage is needed
  * for consistent API between configurations which stores data and which doesn't store data
  * Storage::Memory is for configurations not to store data
  * Storage::Base #save_at_shutdown is for cases not to store data by plugin code
* add #delete method to Storage to clean internal data before #save or so
* create parent directories just before actual #save in Storage::JSON
* Merging this branch is too early because of some bugs

This reverts commit 0b9a79e.
* for plugins to call `Engine.emit` or `Fluent::Engine.emit`
* add method to return user-specified id (or nil)
* and APIs for flushing are not included in this change
* to fix these APIs/hooks w/ implementations of BufferedOutput APIs
@tagomoris
Copy link
Member Author

Rebased.

header['Content-length'] ||= body.bytesize
header['Content-type'] ||= 'text/plain'
if body
header['Content-length'] ||= body.bytesize
Copy link
Contributor

Choose a reason for hiding this comment

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

In RFC 7230, content length header is described as Content-Length.
Should we follow the RFC or keep as-is for backward compatibility?

@tagomoris
Copy link
Member Author

I found some inconsistent changes after rebase, and now, I think that it's inappropriate to push this change as a huge pull-request.
So I'll create some different patches and pull-requests for diffs which should be contained to this.

Anyway, this change is good reference to re-create diffs. Please leave this as is for now.

@tagomoris tagomoris closed this Mar 8, 2016
@tagomoris tagomoris deleted the v0.14 branch March 8, 2016 00:05
@coveralls
Copy link

coveralls commented Nov 14, 2016

Coverage Status

Changes Unknown when pulling 9c58f42 on v0.14 into * on master*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request *Deprecated Label* Use enhancement label in general v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants