Skip to content

Conversation

@sirknightj
Copy link
Contributor

@sirknightj sirknightj commented Apr 11, 2025

Issue #, if available:

What was changed?

  • See Add missing RetryStrategyCallbacks classes #192 for the original details.
  • Adding zeroed (NULL) values to have PIC use the defaults.
  • Mentioned in the linked PR, that overriding (providing custom) values for these fields is not currently supported.

Why was it changed?

  • Avoiding undefined behavior due to non-zeroed memory.

How was it changed?

  • Adding the models from PIC. This corresponds with CPP#1248 (original: CPP#1161 to zero out the struct.

What testing was done for the changes?

  • The new and improved CI was run for these changes, which includes both running the samples and running the unit tests.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

stefankiesz and others added 2 commits April 10, 2025 22:44
* Add logs

* Added classes, successfully built and ran

* Revert "Add logs"

* Remove getKvsRetryStrategyCallbacks

* Add getters for all new members, fix typos, make AutomaticStreamingFlags class

* Fix KvsRetryStrategyType

* Update constructor param names

* Return long members in getters rather than 0 for unsupported members

* Update
final long serviceCallCompletionTimeout, final int logLevel,
final boolean logMetric, final AutomaticStreamingFlags flag) {
mVersion = CLIENT_INFO_CURRENT_VERSION;
mCreateClientTimeout = createClientTimeout;
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be consistent with the native with suffix as ms? or seconds?

@sirknightj sirknightj marked this pull request as ready for review April 15, 2025 18:22
* <p>
* A generic retry strategy
*
* @see <a href="https://github.com/awslabs/amazon-kinesis-video-streams-pic/blob/master/src/utils/include/com/amazonaws/kinesis/video/utils/Include.h">PIC</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@sirknightj sirknightj marked this pull request as draft April 16, 2025 23:05
}

@Override
public void getCurrentRetryAttemptNumberFn(KvsRetryStrategy kvsRetryStrategy, int retryCount) throws ProducerException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem right

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.

3 participants