Skip to content

feat: add isWarmingUp to status response#5047

Merged
acha-bill merged 2 commits intomasterfrom
feat/status-warmup
Mar 14, 2025
Merged

feat: add isWarmingUp to status response#5047
acha-bill merged 2 commits intomasterfrom
feat/status-warmup

Conversation

@acha-bill
Copy link
Contributor

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Add isWarmingUp to /status response.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

@acha-bill acha-bill requested a review from istae as a code owner March 12, 2025 04:12
@acha-bill acha-bill requested a review from martinconic March 12, 2025 04:12
@istae istae linked an issue Mar 13, 2025 that may be closed by this pull request
Copy link
Contributor

@istae istae left a comment

Choose a reason for hiding this comment

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

@acha-bill the issue is about exposing the warmup period of the node you are running in the /status API. We do not need to expose if ours peers are in warmup via the status protocol .

@acha-bill acha-bill force-pushed the feat/status-warmup branch from d50e119 to 6cbdbf5 Compare March 13, 2025 10:26
@acha-bill acha-bill requested a review from istae March 13, 2025 10:27
pkg/node/node.go Outdated
apiService.Mount()
apiService.SetProbe(probe)

apiService.SetWarmupTime(time.Now().Add(o.WarmupTime))
Copy link
Contributor

@istae istae Mar 13, 2025

Choose a reason for hiding this comment

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

here 's the tricky thing, if the postage batches are syncing, then setting this up here becomes inaccurate as we have not run this line yet https://github.com/ethersphere/bee/pull/5047/files#diff-515b2d0691202c4ea5e578ec896258ae9c440c381b608ed1eb7116a27847057dR782

maybe it would help if instead of passing a time to the api pkg, it can be done by toggling a bool externally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Then we can just move it downwards after the sync.
Like where we initialize salud. https://github.com/ethersphere/bee/pull/5047/files#diff-515b2d0691202c4ea5e578ec896258ae9c440c381b608ed1eb7116a27847057dR920

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now that I think of it again, the setter is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the next PR we'll improve how we detect and set the warmup

Copy link
Contributor

@istae istae left a comment

Choose a reason for hiding this comment

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

very nice, much cleaner this time around

@acha-bill acha-bill merged commit 6ab75cb into master Mar 14, 2025
15 checks passed
@acha-bill acha-bill deleted the feat/status-warmup branch March 14, 2025 16:51
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.

Expose whether Bee is in warmup period in a status endpoint

3 participants