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

Add execution time limit #480

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Conversation

VincentLanglet
Copy link
Contributor

No description provided.

@freekmurze
Copy link
Member

Nice, could you maybe also provide a way that user can check if the time limit has been hit. Maybe a function onTimeLimitHit($callable) That passed callable could be called when the time limit has been hit.

Could you also update the docs?

@VincentLanglet
Copy link
Contributor Author

Nice, could you maybe also provide a way that user can check if the time limit has been hit. Maybe a function onTimeLimitHit($callable) That passed callable could be called when the time limit has been hit.

I'm not sure to understand the request. What would be used for ?

This might add some complexity especially because you can hit time limit

  • Because of the total time limit
  • Because of the current time limit

I didn't see a onCrawLimitHit() methods...

Or maybe you would prefer that I split

public function reachedCrawlLimits(): bool

into

public function reachedCrawlLimits(): bool
public function reachedTimeLimits(): bool

?

This way these method will be able to be called publicly to know if the limit is reached.

Could you also update the docs?

Sure, I wanted to get a feedback from the Ci, since I didn't succeed running test on my computer...

@freekmurze
Copy link
Member

Yeah, splitting that method seems like a good a idea 👍

@VincentLanglet
Copy link
Contributor Author

@freekmurze Ci is not triggered, not sure if it requires an action from you ?

@freekmurze
Copy link
Member

CI running now

@VincentLanglet
Copy link
Contributor Author

CI running now

Thanks, tests are green

@freekmurze freekmurze merged commit 344e1b9 into spatie:main Dec 16, 2024
14 checks passed
@freekmurze
Copy link
Member

Thanks!

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