forgejo/runner
11
29
Fork 21
Code Issues 38 Pull requests 1 Actions Packages Releases 26 Activity

Increase fetch interval for Codeberg #141

Merged
earl-warren merged 1 commit from gusted-patch-1 into main 2024-01-24 19:50:16 +00:00
Conversation 5 Commits 1 Files changed 3 +49
Gusted commented 2024-01-05 15:07:19 +00:00
Owner
Copy link

Increase the fetch interval to at least 30 seconds if the runner is configured to be used with Codeberg. This avoids it being rate limited when they do actual work and reduces load on Codeberg.

Increase the fetch interval to at least 30 seconds if the runner is configured to be used with Codeberg. This avoids it being rate limited when they do actual work and reduces load on Codeberg.
Gusted added 1 commit 2024-01-05 15:07:20 +00:00
All checks were successful
/ cascade (pull_request) Has been skipped
Details
checks / build and test (pull_request) Successful in 1m7s
Details
checks / runner exec tests (pull_request) Successful in 54s
Details
/ example-docker-compose (pull_request) Successful in 2m50s
Details
5128324f7d Increase fetch interval 
The current interval is two seconds, which is quite low, so low it can cause runners being rate limited when they do actual 'work'. For an instance like Codeberg that has a moderate amount of runners active the numbers add up in activity and logs, therefor increasing the default interval.
earl-warren commented 2024-01-05 23:04:59 +00:00
Owner
Copy link

I'm not sure what the impact will be on existing workflows & scripts that do not expect such a significant delay. People trying to launch runners for the first time will also likely be confused into thinking they do not work because of this delay.

However I think it would be fine to have a new setting that provides different defaults for known public instances such as codeberg. The runners I configured for Codeberg run with a 60sec delay and a 30sec timeout and they work just fine.

I'm not sure what the impact will be on existing workflows & scripts that do not expect such a significant delay. People trying to launch runners for the first time will also likely be confused into thinking they do not work because of this delay. However I think it would be fine to have a new setting that provides different defaults for known public instances such as codeberg. The runners I configured for Codeberg run with a 60sec delay and a 30sec timeout and they work just fine.
Gusted force-pushed gusted-patch-1 from 5128324f7d to a18e8cf51c 2024-01-08 16:16:06 +00:00 Compare
Gusted changed title from Increase fetch interval to Increase fetch interval for Codeberg 2024-01-08 16:16:27 +00:00
Gusted commented 2024-01-08 16:17:22 +00:00
Author
Owner
Copy link

I've now implemented it as such, it will only increase the fetch interval when the runner is configured to work on Codeberg with a info message telling it did so.

I've now implemented it as such, it will only increase the fetch interval when the runner is configured to work on Codeberg with a info message telling it did so.
earl-warren reviewed 2024-01-08 17:39:24 +00:00
internal/pkg/config/config.go Outdated
@ -70,1 +70,4 @@
// Tune the config settings accordingly to the Forgejo instance that will be used.
func (c *Config) Tune(instanceURL string) {
if instanceURL == "https://codeberg.org" {
earl-warren commented 2024-01-08 17:39:24 +00:00
Owner
Copy link

I'm willing to add a test that verifies this works as it should in https://code.forgejo.org/forgejo/end-to-end to complement this change.

In order to do that I would need the public instance to be from the config file rather than hardcoded so I can setup the conditions in the test environment and verify the delay is enforced as it should. With that this change can be merged and will be released as soon as I complete the associated test.

Does that work for you?

I'm willing to add a test that verifies this works as it should in https://code.forgejo.org/forgejo/end-to-end to complement this change. In order to do that I would need the public instance to be from the config file rather than hardcoded so I can setup the conditions in the test environment and verify the delay is enforced as it should. With that this change can be merged and will be released as soon as I complete the associated test. Does that work for you?
Gusted commented 2024-01-08 21:51:37 +00:00
Author
Owner
Copy link

Do you want to check that it actually fetches every 30 seconds? This also could be a simple unit test right?

Do you want to check that it actually fetches every 30 seconds? This also could be a simple unit test right?
earl-warren commented 2024-01-08 22:01:41 +00:00
Owner
Copy link

Yes, it could be a simple unit test. The runner is vastly untested and I was proposing that to save you the trouble of figuring out how to test that.

Yes, it could be a simple unit test. The runner is vastly untested and I was proposing that to save you the trouble of figuring out how to test that.
Gusted commented 2024-01-09 20:02:53 +00:00
Author
Owner
Copy link

The configured workflow doesn't run any unit tests at this moment?

The configured workflow doesn't run any unit tests at this moment?
earl-warren commented 2024-01-09 21:22:16 +00:00
Owner
Copy link

It does, only they do not cover much.

It does, only they do not cover much.
Gusted commented 2024-01-21 22:03:51 +00:00
Author
Owner
Copy link

a18e8cf51c..db2213254d

https://code.forgejo.org/forgejo/runner/compare/a18e8cf51c9035c126bd0f31e58ef5e4cef7af61..db2213254d4243199ff1ca08a2cde7342346e7e6
Gusted marked this conversation as resolved
Gusted force-pushed gusted-patch-1 from a18e8cf51c to db2213254d 2024-01-21 22:03:34 +00:00 Compare
earl-warren approved these changes 2024-01-21 22:52:51 +00:00
earl-warren commented 2024-01-21 22:53:47 +00:00
Owner
Copy link

I'll trigger and verify the cascading pr result.

I'll trigger and verify the cascading pr result.
cascading-pr commented 2024-01-24 19:22:30 +00:00
First-time contributor
Copy link

cascading-pr updated at actions/setup-forgejo#113

cascading-pr updated at https://code.forgejo.org/actions/setup-forgejo/pulls/113
earl-warren commented 2024-01-24 19:32:55 +00:00
Owner
Copy link

✅ runner used with all end-to-end tests at forgejo/end-to-end#73

✅ runner used with all end-to-end tests at https://code.forgejo.org/forgejo/end-to-end/pulls/73
earl-warren merged commit c4e8c08065 into main 2024-01-24 19:50:16 +00:00
earl-warren deleted branch gusted-patch-1 2024-01-24 19:50:17 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
earl-warren
Labels
Clear labels   
Kind/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No labels
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: forgejo/runner#141
No description provided.
Delete branch "gusted-patch-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?