Skip to content

[Feature] Add clock skew computation and unit test - #3898

Merged
kai-ion merged 1 commit into
mainfrom
timechange
Aug 31, 2026
Merged

[Feature] Add clock skew computation and unit test#3898
kai-ion merged 1 commit into
mainfrom
timechange

Conversation

@kai-ion

@kai-ion kai-ion commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Issue #, if available:

Description of changes:
Add clock skew computation and unit test

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

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

@kai-ion kai-ion changed the title Add clock skew computation and unit test [Feature] Add clock skew computation and unit test Aug 24, 2026
@kai-ion
kai-ion marked this pull request as ready for review August 26, 2026 19:43
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
Comment thread tests/aws-cpp-sdk-core-tests/utils/ClockSkewTest.cpp Outdated
@kai-ion
kai-ion force-pushed the timechange branch 2 times, most recently from c299fbb to 6214afe Compare August 28, 2026 14:51
Comment thread src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h Outdated
// Retryable when the applied skew is off from the observed skew by more than the threshold.
if (candidate.has_value())
{
const std::chrono::milliseconds delta = attemptSkew - candidate.value();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ok im sort of split on this, nice to have wont block it, the reason you are doing all of this is because std::chrono::abs is in c++17. i sort of hate we are forced to do this the wrong way. so a suggestion. in aws/core/utils/local you could add Chrono.h and have something like

namespace Aws {
#if defined(__cpp_lib_chrono) && __cpp_lib_chrono >= 201510L
using std::chrono::abs;
using std::chrono::ceil;
using std::chrono::floor;
using std::chrono::round;
#else
// TODO: delete this branch once the minimum standard is C++17.
template <class Rep, class Period>
constexpr std::enable_if_t<std::numeric_limits<Rep>::is_signed,
                           std::chrono::duration<Rep, Period>>
abs(std::chrono::duration<Rep, Period> d) {
    return d >= d.zero() ? d : -d;
}
#endif
}

then here you could just call Aws::chrono::abs, and if you compile with 17 you get the feature, if you compile without you get the shim implementation.

@kai-ion
kai-ion force-pushed the timechange branch 2 times, most recently from 42e8b23 to 68f9565 Compare August 28, 2026 19:40
Add clock skew correction computation and unit test
@kai-ion
kai-ion enabled auto-merge August 31, 2026 14:45
@kai-ion
kai-ion added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit d82ba80 Aug 31, 2026
3 of 6 checks passed
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.

2 participants