logstash-plugins / logstash-input-salesforce

Logstash input for pulling objects from salesforce
Apache License 2.0
16 stars 29 forks source link

make this plugin compatible with logstash 5.x #6

Closed jsvd closed 7 years ago

guyboertje commented 7 years ago

one small teeny thing. Changelog reads Update dependency to logstash-core-plugin-api > 1.60 < 2.0 should this be < 3.0?

jsvd commented 7 years ago

fixed build failing due to https://github.com/bundler/bundler/issues/5349

guyboertje commented 7 years ago

I feel, this

            case field_type
            when 'datetime'
              event.set(event_key, LogStash::Timestamp.parse(value))
            when 'date'
              event.set(event_key, LogStash::Timestamp.parse(value))
            else
              event.set(event_key, value)
            end

should be

            case field_type
            when 'datetime', 'date'
              event.set(event_key, LogStash::Timestamp.parse(value))
            else
              event.set(event_key, value)
            end

Your call. It does not appear that the VCR fixtures would test the datetime or date case clause. Maybe verify that the specs do execute that path?

guyboertje commented 7 years ago

LGTM (whether you choose to change the case statement or not)

elasticsearch-bot commented 7 years ago

João Duarte merged this into the following branches!

Branch Commits
master 9189fcfb32c0e82fcd461d8cea94da4388617883