Skip to content

Conversation

@mjc1283
Copy link
Contributor

@mjc1283 mjc1283 commented Aug 9, 2019

Summary

Add/change validation for maxQueueSize and flushInterval inputs to AbstractEventProcessor. When invalid, ignore and use default values.

Test plan

New unit tests

Issues

OASIS-5167

@mjc1283 mjc1283 requested review from a team and mikeproeng37 August 9, 2019 23:16
@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.597% when pulling cedb007 on mcarroll/OASIS-5167 into 99f4f8a on master.

Copy link
Contributor

@aliabbasrizvi aliabbasrizvi left a comment

Choose a reason for hiding this comment

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

Looks good.

}

const MIN_FLUSH_INTERVAL = 100
const DEFAULT_FLUSH_INTERVAL = 30000
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment about what this value is? i.e. this does not talk about unit so a simple comment about this being 30 seconds will be useful I feel.


maxQueueSize = Math.floor(maxQueueSize)
if (maxQueueSize < 1) {
logger.warn(`Invalid maxQueueSize, defaulting to ${DEFAULT_MAX_QUEUE_SIZE}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's log the invalid maxQueueSize value as well?

@mjc1283 mjc1283 merged commit c479d40 into master Aug 12, 2019
@mjc1283 mjc1283 deleted the mcarroll/OASIS-5167 branch August 12, 2019 16:00
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.

4 participants