Skip to content

fixes #2689 Make the external url of cache server configurable if necessary #2690

Merged
mergify[bot] merged 3 commits intonektos:masterfrom
Zoupers:master
Mar 5, 2025
Merged

fixes #2689 Make the external url of cache server configurable if necessary #2690
mergify[bot] merged 3 commits intonektos:masterfrom
Zoupers:master

Conversation

@Zoupers
Copy link
Contributor

@Zoupers Zoupers commented Mar 3, 2025 •
edited
Loading

According to this pr, fix the test and optimize the code.
Fix #2689 .

linw1995 and others added 2 commits March 4, 2025 00:16
@linw1995 @Zoupers
Make the external url of cache server configurable if necessary
d9f381e 
Signed-off-by: Zoupers <qy@zouper.cn>
@Zoupers
optimize code and doc and fix test and add test
5eb1e87 
Signed-off-by: Zoupers <qy@zouper.cn>
@Zoupers Zoupers changed the title Make the external url of cache server configurable if necessary fix #2689 Make the external url of cache server configurable if necessary Mar 3, 2025
@Zoupers Zoupers changed the title fix #2689 Make the external url of cache server configurable if necessary fixes #2689 Make the external url of cache server configurable if necessary Mar 3, 2025
@ChristopherHX
Copy link
Contributor

ChristopherHX commented Mar 3, 2025

I wanted to deploy the cache server as a separate Deployment/Service from the runner, so I had set 0.0.0.0 as the host for the cache server.

cache:
  enabled: true
  host: "0.0.0.0"
  port: 80

If you have the same usecase, we should add an subcommand to the act cli to start the cache server independently as well.

Per https://gitea.com/gitea/act_runner/issues/668#issuecomment-968966

I make the way free for a merge, looks good to me

ChristopherHX
ChristopherHX previously approved these changes Mar 3, 2025
View reviewed changes
ChristopherHX
ChristopherHX reviewed Mar 3, 2025
View reviewed changes
@codecov
Copy link

codecov bot commented Mar 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.41%. Comparing base (5a80a04) to head (1be6fae).
Report is 186 commits behind head on master.

Files with missing lines Patch % Lines
pkg/artifactcache/handler.go 70.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #2690       +/-   ##
===========================================
+ Coverage   61.56%   74.41%   +12.85%     
===========================================
  Files          53       72       +19     
  Lines        9002    11062     +2060     
===========================================
+ Hits         5542     8232     +2690     
+ Misses       3020     2193      -827     
- Partials      440      637      +197

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Zoupers
optimize code and adjust description
1be6fae 
Signed-off-by: Zoupers <qy@zouper.cn>
@Zoupers Zoupers requested a review from ChristopherHX March 4, 2025 14:51
ChristopherHX
ChristopherHX approved these changes Mar 4, 2025
View reviewed changes
Copy link
Contributor

@ChristopherHX ChristopherHX left a comment

Choose a reason for hiding this comment

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

LGTM, but keep in mind this needs another approval

@mergify mergify bot merged commit 49710c8 into nektos:master Mar 5, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cplee cplee cplee approved these changes

@ChristopherHX ChristopherHX ChristopherHX approved these changes

Assignees

No one assigned

Labels

size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

@actions/cache with external cache server connecting wrong IP (0.0.0.0)

4 participants

@Zoupers @ChristopherHX @cplee @linw1995