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.

Sign up for GitHub

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

Jump to bottom

Bug 1519213 - NoMethodError if remote_syslog enabled remote_syslog_ta… #820

Merged
merged 1 commit into from
Dec 8, 2017
Merged

Bug 1519213 - NoMethodError if remote_syslog enabled remote_syslog_ta… #820

merged 1 commit into from
Dec 8, 2017

Conversation

nhosoi
Copy link
Contributor

@nhosoi nhosoi commented Dec 6, 2017

…g_key

fluent-plugin-remote-syslog: replacing tag_key with @tag_key
adding rescue around record[@tag_key]
Note: out_syslog.rb and out_syslog_buffered.rb are copied from
https://github.com/docebo/fluent-plugin-remote-syslog and
locally modified. Once the changes made on the files are
accepted by the plugin, out_syslog.rb and out_syslog_buffered.rb
are to be removed and the changes on Dockerfile* are to be undone.

Adding test cases for tag_key to remote_syslog.sh

@nhosoi
Bug 1519213 - NoMethodError if remote_syslog enabled remote_syslog_ta…
1cea2e1 
…g_key

fluent-plugin-remote-syslog: replacing tag_key with @tag_key
                             adding rescue around record[@tag_key]
Note: out_syslog.rb and out_syslog_buffered.rb are copied from
      https://github.com/docebo/fluent-plugin-remote-syslog and
      locally modified.  Once the changes made on the files are
      accepted by the plugin, out_syslog.rb and out_syslog_buffered.rb
      are to be removed and the changes on Dockerfile* are to be undone.

Adding test cases for tag_key to remote_syslog.sh
@nhosoi nhosoi requested a review from richm December 6, 2017 22:22
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 6, 2017
@richm
Copy link
Contributor

richm commented Dec 6, 2017

Did you also submit a patch upstream to https://github.com/docebo/fluent-plugin-remote-syslog?

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 6, 2017

Yes, this is the PR.
docebo/fluent-plugin-remote-syslog#12

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 6, 2017 •
edited
Loading

Not sure what is wrong, but building failed in the upstream fluent-plugin-remote-syslog...

continuous-integration/travis-ci/pr — The Travis CI build could not complete due to an error
https://travis-ci.org/docebo/fluent-plugin-remote-syslog/builds/312670193?utm_source=github_status&utm_medium=notification

The previous PR also failed in the very similar manner...
docebo/fluent-plugin-remote-syslog#11

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 6, 2017

/retest

1 similar comment
@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 7, 2017

/retest

richm
richm approved these changes Dec 7, 2017
View reviewed changes
Copy link
Contributor

@richm richm left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2017
@richm
Copy link
Contributor

richm commented Dec 7, 2017

/cherrypick release-3.7

@openshift-cherrypick-robot
Copy link

@richm: once the present PR merges, I will cherry-pick it on top of release-3.7 in a new PR and assign it to you.

In response to this:

/cherrypick release-3.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@richm richm added backport/3.7 component/fluentd kind/bug Categorizes issue or PR as related to a bug. release/3.9 labels Dec 8, 2017
@richm
Copy link
Contributor

richm commented Dec 8, 2017

why isn't this merging?

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 8, 2017

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit b223dc2 into openshift:master Dec 8, 2017
@openshift-cherrypick-robot
Copy link

@richm: new pull request created: #830

In response to this:

/cherrypick release-3.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Dec 8, 2017
Merged
openshift-merge-robot added a commit that referenced this pull request Dec 8, 2017
@openshift-merge-robot
Merge pull request #830 from openshift-cherrypick-robot/cherry-pick-8…
56c2e57 
…20-to-release-3.7

Automatic merge from submit-queue.

Automated cherry-pick of #820 on release-3.7

This is an automated cherry-pick of #820

/assign richm
@portante
Copy link
Contributor

portante commented Dec 8, 2017

Not sure this is legal to do. We can't just copy source code into our repository like this if it is licensed. I think that code is under the MIT license, at least in the gemspec it says MIT.

@richm
Copy link
Contributor

richm commented Dec 8, 2017

I've asked legal for advice

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 8, 2017

@portante, @richm,

I think that code is under the MIT license, at least in the gemspec it says MIT.
Good to hear that.
A community member submitted this pr on 2017-03-22, but there has been no activities in the pr since then.
docebo/fluent-plugin-remote-syslog#9
So, even if the plugin does not have a LICENSE file nor a license section in each source file, the plugin is "protected"?

@portante
Copy link
Contributor

portante commented Dec 8, 2017

@nhosoi, better safe than sorry and check with legal.

@nhosoi
Copy link
Contributor Author

nhosoi commented Dec 8, 2017

@porante, you are right. I've opened this pr following the suggestion from the Legal team.
#836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richm richm richm approved these changes

Assignees

@richm richm

Labels
backport/completed backport/3.7 component/fluentd kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release/3.9 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nhosoi @richm @openshift-cherrypick-robot @openshift-merge-robot @portante @openshift-ci-robot