Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Add Configs, tests, related PR#15 #18

Merged
merged 5 commits into from
Aug 18, 2017
Merged

Add Configs, tests, related PR#15 #18

merged 5 commits into from
Aug 18, 2017

Conversation

fernando-torterolo
Copy link
Contributor

Add configs and test related to @markglh 's comments:
#15

@coveralls
Copy link

coveralls commented Aug 16, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.9%) to 85.795% when pulling be652dc on feature/kinesesPropertiesTest into 8e2cea7 on master.

@markglh markglh requested review from agaro1121 and j-potts August 16, 2017 15:22
@markglh
Copy link
Contributor

markglh commented Aug 16, 2017 •
edited
Loading

Addresses: #16

markglh
markglh previously approved these changes Aug 16, 2017
View reviewed changes
Copy link
Contributor

@markglh markglh 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, interesting that code coverage decreased.

Perhaps we should exclude the configs?

Approving but we need to fix the CI issues before merging.

@fernando-oktana - Did you by any chance compare the available properties in the KPL & KCL with what we're exposing on our reference.conf? I can close the related issues is so, otherwise I'll raise an issue so we don't forget. I want to make sure that no other new ones have been added that we missed

@fernando-torterolo
Copy link
Contributor Author

@markglh Thanks!
Perhaps we should exclude the configs? > +1
Approving but we need to fix the CI issues before merging. > Yes, It seems test failed using scala 2.12 https://travis-ci.org/WW-Digital/reactive-kinesis/builds/265181851, I need to figure out

@fernando-oktana - Did you by any chance compare the available properties in the KPL & KCL with what we're exposing on our reference.conf? I can close the related issues is so, otherwise I'll raise an issue so we don't forget. I want to make sure that no other new ones have been added that we missed
KLP: will add missing fields

property status
private boolean aggregationEnabled = true; ok
private long aggregationMaxCount = 4294967295L; ok
private long aggregationMaxSize = 51200L; ok
private String cloudwatchEndpoint = ""; ok
private long cloudwatchPort = 443L; ok
private long collectionMaxCount = 500L; ok
private long collectionMaxSize = 5242880L; ok
private long connectTimeout = 6000L; ok
private long credentialsRefreshDelay = 5000L; Missing
private boolean enableCoreDumps = false; ok
private boolean failIfThrottled = false; ok
private String kinesisEndpoint = ""; ok
private long kinesisPort = 443L; ok
private String logLevel = "info"; ok
private long maxConnections = 24L; ok
private String metricsGranularity = "shard"; ok
private String metricsLevel = "detailed"; ok
private String metricsNamespace = "KinesisProducerLibrary"; ok
private long metricsUploadDelay = 60000L; ok
private long minConnections = 1L; ok
private String nativeExecutable = ""; Missing
private long rateLimit = 150L; ok
private long recordMaxBufferedTime = 100L; ok
private long recordTtl = 30000L; ok
private String region = ""; ok
private long requestTimeout = 6000L; ok
private String tempDirectory = ""; Missing
private boolean verifyCertificate = true; ok
private ThreadingModel threadingModel = ThreadingModel.PER_REQUEST; Missing
private int threadPoolSize = 0; Missing

KCL: TODO

@coveralls
Copy link

coveralls commented Aug 16, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 86.648% when pulling 12c3a1f on feature/kinesesPropertiesTest into 8e2cea7 on master.

@fernando-torterolo
Copy link
Contributor Author

fernando-torterolo commented Aug 16, 2017 •
edited
Loading

KCL

property status
private String applicationName; ok, set on producer config
private String tableName; Missing
private String streamName; ok, set on producer config
private String kinesisEndpoint; ok
private String dynamoDBEndpoint; Missing
private InitialPositionInStream initialPositionInStream; ok
private AWSCredentialsProvider kinesisCredentialsProvider; no setter defined
private AWSCredentialsProvider dynamoDBCredentialsProvider; no setter defined
private AWSCredentialsProvider cloudWatchCredentialsProvider; no setter defined
private long failoverTimeMillis; ok
private String workerIdentifier; ok
private long shardSyncIntervalMillis; ok
private int maxRecords; ok
private long idleTimeBetweenReadsInMillis; ok
private boolean callProcessRecordsEvenForEmptyRecordList; ok
private long parentShardPollIntervalMillis; ok
private boolean cleanupLeasesUponShardCompletion; ok
private ClientConfiguration kinesisClientConfig; Missing
private ClientConfiguration dynamoDBClientConfig; Missing
private ClientConfiguration cloudWatchClientConfig; Missing
private long taskBackoffTimeMillis; ok
private long metricsBufferTimeMillis; ok
private int metricsMaxQueueSize; ok
private MetricsLevel metricsLevel; ok
private Set metricsEnabledDimensions; ok
private boolean validateSequenceNumberBeforeCheckpointing; ok
private String regionName; ok
private int maxLeasesForWorker; ok
private int maxLeasesToStealAtOneTime; ok
private int initialLeaseTableReadCapacity; ok
private int initialLeaseTableWriteCapacity; ok
private InitialPositionInStreamExtended initialPositionInStreamExtended; protected
private boolean skipShardSyncAtWorkerInitializationIfLeasesExist; Missing
private ShardPrioritization shardPrioritization; ---

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.648% when pulling 4cf6e86 on feature/kinesesPropertiesTest into 8e2cea7 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.648% when pulling 4cf6e86 on feature/kinesesPropertiesTest into 8e2cea7 on master.

Fernando Torterolo added 2 commits August 17, 2017 15:23
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.648% when pulling 3209cb4 on feature/kinesesPropertiesTest into 8e2cea7 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.648% when pulling 3209cb4 on feature/kinesesPropertiesTest into 8e2cea7 on master.

markglh
markglh approved these changes Aug 18, 2017
View reviewed changes
Copy link
Contributor

@markglh markglh left a comment

Choose a reason for hiding this comment

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

👍 Thanks!

@markglh markglh merged commit 4f84e22 into master Aug 18, 2017
@agaro1121 agaro1121 deleted the feature/kinesesPropertiesTest branch February 7, 2018 03:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@j-potts j-potts j-potts approved these changes

@markglh markglh markglh approved these changes

@agaro1121 agaro1121 Awaiting requested review from agaro1121

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fernando-torterolo @coveralls @markglh @j-potts