fixes #2689 Make the external url of cache server configurable if necessary #2690
Conversation
Signed-off-by: Zoupers <qy@zouper.cn>
Signed-off-by: Zoupers <qy@zouper.cn>
Zoupers
changed the title
Zoupers
changed the title
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 |
| if advertiseURL != "" { | ||
| h.advertiseURL = advertiseURL | ||
| } else if outboundIP != "" { |
There was a problem hiding this comment.
I would still assign h.outboundIP here even if advertiseURL is passed, while it looks to be not necessary
| if advertiseURL != "" { | |
| h.advertiseURL = advertiseURL | |
| } else if outboundIP != "" { | |
| if advertiseURL != "" { | |
| h.advertiseURL = advertiseURL | |
| } | |
| if outboundIP != "" { |
There was a problem hiding this comment.
OK, I have hesitated whether to assign values separately, because if in the future the function of outboundIP has changed to bind specific address, this change will be not cool. I will take your suggestion.
| @@ -118,6 +118,7 @@ func createRootCommand(ctx context.Context, input *Input, version string) *cobra | |||
| rootCmd.PersistentFlags().BoolVarP(&input.noSkipCheckout, "no-skip-checkout", "", false, "Use actions/checkout instead of copying local files into container") | |||
| rootCmd.PersistentFlags().BoolVarP(&input.noCacheServer, "no-cache-server", "", false, "Disable cache server") | |||
| rootCmd.PersistentFlags().StringVarP(&input.cacheServerPath, "cache-server-path", "", filepath.Join(CacheHomeDir, "actcache"), "Defines the path where the cache server stores caches.") | |||
| rootCmd.PersistentFlags().StringVarP(&input.cacheServerAdvertiseURL, "cache-server-advertise-url", "", "", "Defines the URL for advertising the cache server behind a proxy. e.g.: https://act-cache-server.example.com. Be careful that there is no trailing slash.") | |||
There was a problem hiding this comment.
I like --cache-server-external-url more, including this makes cache-server-addr to be ignored.
Unrelated, based on the code the description of Defines the address to which the cache server binds feels wrong
There was a problem hiding this comment.
OK, I will adjust the description and option.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2690 +/- ##
===========================================
+ Coverage 61.56% 74.42% +12.85%
===========================================
Files 53 72 +19
Lines 9002 11061 +2059
===========================================
+ Hits 5542 8232 +2690
+ Misses 3020 2192 -828
- Partials 440 637 +197 ☔ View full report in Codecov by Sentry. |
According to this pr, fix the test and optimize the code.
Fix #2689 .