Skip to content

Conversation

@nkomonen-amazon
Copy link
Contributor

Creates our own version of an Interval, similar to how we have our own Timeout


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Use this instead of globals.clock.setInterval since it
will give us some more control when needed

Signed-off-by: nkomonen-amazon <[email protected]>
@nkomonen-amazon nkomonen-amazon requested a review from a team as a code owner October 16, 2024 15:01
@github-actions
Copy link

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

@hayemaxi
Copy link
Contributor

QQ: How are these classes different from globals.clock.setInterval and globals.clock.setTimeout?

@nkomonen-amazon
Copy link
Contributor Author

@hayemaxi it behaves the same, but gives us some extra control and allows us to add features to it.

An example is this nextCompletion() method which is not available ootb with globals.clock.setInterval.

It can also be thought of as how we wrap globalState or setContext

@justinmk3
Copy link
Contributor

justinmk3 commented Oct 16, 2024

This new Interval class doesn't have a docstring, so I assume the only difference vs Timeout is that it repeats. Why can't that simply be a feature added to Timeout (if it doesn't already exist)?

@nkomonen-amazon
Copy link
Contributor Author

@justinmk3 most of the methods in Timeout dont make sense in the context of an Interval:

  • remainingTime, intervals run infinitely
  • completed, intervals run infinitely
  • refresh, intervals automatically refresh
  • promisify, you don't wait for an interval to complete
  • cancellationToken, it doesn't timeout by default

Over time if we see a use for something like elapsedTime, we could look to generalize that between Timeout and Interval

@nkomonen-amazon nkomonen-amazon merged commit e900737 into aws:master Oct 19, 2024
22 of 24 checks passed
@nkomonen-amazon nkomonen-amazon deleted the intervalClass branch October 19, 2024 03:31
tverney pushed a commit to tverney/aws-toolkit-vscode that referenced this pull request Oct 21, 2024
Creates our own version of an Interval, similar to how we have our own
Timeout

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

Signed-off-by: nkomonen-amazon <[email protected]>
@justinmk3
Copy link
Contributor

justinmk3 commented Feb 12, 2025

remainingTime, intervals run infinitely
completed, intervals run infinitely

That can be optional. Not an inherent property of either (except how "Interval" is currently defined in this PR).

  • refresh, intervals automatically refresh

That is optional. And a caller may want to do an "eager" refresh.

  • promisify, you don't wait for an interval to complete
  • cancellationToken, it doesn't timeout by default

I don't understand these. But perhaps they also make sense as optional features of Timeout with "interval behavior".

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