Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.x] Ignore offline servers (continued) #355

Merged
merged 11 commits into from
May 1, 2024

Conversation

JustinElst
Copy link
Contributor

This pull request continues the work in [1.x] Ignore offline servers.
Thanks leventdev for starting this.
With this pull-request I wanted to complete the request made by in jessarcher in her comment.

@taylorotwell taylorotwell marked this pull request as draft April 15, 2024 15:20
@jessarcher
Copy link
Member

jessarcher commented Apr 16, 2024

Thanks, @JustinElst and @leventdev!

I changed the config to be in seconds rather than minutes to be consistent with other config values like thresholds. I also removed the additional 30-second grace period, as I don't think it made as much sense with the config in seconds.

@jessarcher jessarcher self-assigned this Apr 16, 2024
config/pulse.php Outdated Show resolved Hide resolved
@timacdonald timacdonald force-pushed the ignore-offline-servers branch 6 times, most recently from 95fe9de to 2bd9d5e Compare May 1, 2024 02:06
@timacdonald timacdonald marked this pull request as ready for review May 1, 2024 02:07
@timacdonald
Copy link
Member

timacdonald commented May 1, 2024

API:

<!-- Ignore servers after 600 seconds: -->

<livewire:pulse.servers cols="full" ignore-after="600" />

<!-- Ignore servers after 2 hours: -->

<livewire:pulse.servers cols="full" ignore-after="2 hours" />

@timacdonald timacdonald marked this pull request as draft May 1, 2024 02:26
Comment on lines +74 to +90
/**
* Determine if the system should be ignored.
*
* @param object{ timestamp: int, key: string, value: string } $system
*/
protected function ignoreSystem(object $system): bool
{
if ($this->ignoreAfter === null) {
return false;
}

$ignoreAfter = is_numeric($this->ignoreAfter)
? (int) $this->ignoreAfter
: CarbonInterval::createFromDateString($this->ignoreAfter)->totalSeconds;

return CarbonImmutable::createFromTimestamp($system->timestamp)->addSeconds($ignoreAfter)->isPast();
}
Copy link
Member

Choose a reason for hiding this comment

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

I've called this ignoreSystem to differentiate it from our shared ignore method on the Ignores trait.

Comment on lines +19 to +22
use InteractsWithTime;

public int|string|null $ignoreAfter = null;

Copy link
Member

Choose a reason for hiding this comment

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

Livewire does not support DateInterval as a prop value, so I've excluded it from the type here.

Due to the way Livewire serializes values, we also cannot use now()->addSeconds(60) in the prop value.

@timacdonald timacdonald marked this pull request as ready for review May 1, 2024 04:18
@timacdonald
Copy link
Member

Docs: laravel/docs#9622

@taylorotwell taylorotwell merged commit 594e921 into laravel:1.x May 1, 2024
16 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.

5 participants