STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #8931 from cli/wm/run-log-cache-stronger-abstraction

Browse source 
Create stronger run log cache abstraction
This commit is contained in:
William Martin 2024-04-05 16:00:28 +02:00 • committed by GitHub
commit b6239238c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 115 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

6
internal/config/config.go
View file

@ -36,6 +36,8 @@ type Config interface {
Write() error
Migrate(Migration) error
CacheDir() string
Aliases() *AliasConfig
Authentication() *AuthConfig
Browser(string) string
@ -191,6 +193,10 @@ func (c *cfg) Migrate(m Migration) error {
return nil
}
func (c *cfg) CacheDir() string {
return ghConfig.CacheDir()
}
func defaultFor(key string) (string, bool) {
for _, co := range ConfigOptions() {
if co.Key == key {

37
internal/config/config_mock.go
View file

@ -26,6 +26,9 @@ var _ Config = &ConfigMock{}
// BrowserFunc: func(s string) string {
// panic("mock out the Browser method")
// },
// CacheDirFunc: func() string {
// panic("mock out the CacheDir method")
// },
// EditorFunc: func(s string) string {
// panic("mock out the Editor method")
// },
@ -72,6 +75,9 @@ type ConfigMock struct {
// BrowserFunc mocks the Browser method.
BrowserFunc func(s string) string
// CacheDirFunc mocks the CacheDir method.
CacheDirFunc func() string
// EditorFunc mocks the Editor method.
EditorFunc func(s string) string
@ -115,6 +121,9 @@ type ConfigMock struct {
// S is the s argument value.
S string
}
// CacheDir holds details about calls to the CacheDir method.
CacheDir []struct {
}
// Editor holds details about calls to the Editor method.
Editor []struct {
// S is the s argument value.
@ -171,6 +180,7 @@ type ConfigMock struct {
lockAliases sync.RWMutex
lockAuthentication sync.RWMutex
lockBrowser sync.RWMutex
lockCacheDir sync.RWMutex
lockEditor sync.RWMutex
lockGetOrDefault sync.RWMutex
lockGitProtocol sync.RWMutex
@ -269,6 +279,33 @@ func (mock *ConfigMock) BrowserCalls() []struct {
return calls
}
// CacheDir calls CacheDirFunc.
func (mock *ConfigMock) CacheDir() string {
if mock.CacheDirFunc == nil {
panic("ConfigMock.CacheDirFunc: method is nil but Config.CacheDir was just called")
}
callInfo := struct {
}{}
mock.lockCacheDir.Lock()
mock.calls.CacheDir = append(mock.calls.CacheDir, callInfo)
mock.lockCacheDir.Unlock()
return mock.CacheDirFunc()
}
// CacheDirCalls gets all the calls that were made to CacheDir.
// Check the length with:
//
// len(mockedConfig.CacheDirCalls())
func (mock *ConfigMock) CacheDirCalls() []struct {
} {
var calls []struct {
}
mock.lockCacheDir.RLock()
calls = mock.calls.CacheDir
mock.lockCacheDir.RUnlock()
return calls
}
// Editor calls EditorFunc.
func (mock *ConfigMock) Editor(s string) string {
if mock.EditorFunc == nil {

3
internal/config/stub.go
View file

@ -77,6 +77,9 @@ func NewFromString(cfgStr string) *ConfigMock {
val, _ := cfg.GetOrDefault("", versionKey)
return val
}
mock.CacheDirFunc = func() string {
return cfg.CacheDir()
}
return mock
}

95
pkg/cmd/run/view/view.go
View file

@ -28,36 +28,53 @@ import (
"github.com/spf13/cobra"
)
type runLogCache interface {
Exists(string) bool
Create(string, io.Reader) error
Open(string) (*zip.ReadCloser, error)
type RunLogCache struct {
cacheDir string
}
type rlc struct{}
func (rlc) Exists(path string) bool {
if _, err := os.Stat(path); err != nil {
return false
func (c RunLogCache) Exists(key string) (bool, error) {
_, err := os.Stat(c.filepath(key))
if err == nil {
return true, nil
}
return true
if errors.Is(err, os.ErrNotExist) {
return false, nil
}
return false, fmt.Errorf("checking cache entry: %v", err)
}
func (rlc) Create(path string, content io.Reader) error {
err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return fmt.Errorf("could not create cache: %w", err)
func (c RunLogCache) Create(key string, content io.Reader) error {
if err := os.MkdirAll(filepath.Dir(c.cacheDir), 0755); err != nil {
return fmt.Errorf("creating cache directory: %v", err)
}
out, err := os.Create(path)
out, err := os.Create(c.filepath(key))
if err != nil {
return err
return fmt.Errorf("creating cache entry: %v", err)
}
defer out.Close()
_, err = io.Copy(out, content)
return err
if _, err := io.Copy(out, content); err != nil {
return fmt.Errorf("writing cache entry: %v", err)
}
return nil
}
func (rlc) Open(path string) (*zip.ReadCloser, error) {
return zip.OpenReader(path)
func (c RunLogCache) Open(key string) (*zip.ReadCloser, error) {
r, err := zip.OpenReader(c.filepath(key))
if err != nil {
return nil, fmt.Errorf("opening cache entry: %v", err)
}
return r, nil
}
func (c RunLogCache) filepath(key string) string {
return filepath.Join(c.cacheDir, fmt.Sprintf("run-log-%s.zip", key))
}
type ViewOptions struct {
@ -66,7 +83,7 @@ type ViewOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
Prompter shared.Prompter
RunLogCache runLogCache
RunLogCache RunLogCache
RunID string
JobID string
@ -85,12 +102,11 @@ type ViewOptions struct {
func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command {
opts := &ViewOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
Prompter: f.Prompter,
Now: time.Now,
Browser: f.Browser,
RunLogCache: rlc{},
IO: f.IOStreams,
HttpClient: f.HttpClient,
Prompter: f.Prompter,
Now: time.Now,
Browser: f.Browser,
}
cmd := &cobra.Command{
@ -126,6 +142,15 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
config, err := f.Config()
if err != nil {
return err
}
opts.RunLogCache = RunLogCache{
cacheDir: config.CacheDir(),
}
if len(args) == 0 && opts.JobID == "" {
if !opts.IO.CanPrompt() {
return cmdutil.FlagErrorf("run or job ID required when not running interactively")
@ -430,10 +455,14 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) {
return resp.Body, nil
}
func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run, attempt uint64) (*zip.ReadCloser, error) {
filename := fmt.Sprintf("run-log-%d-%d.zip", run.ID, run.StartedTime().Unix())
filepath := filepath.Join(os.TempDir(), "gh-cli-cache", filename)
if !cache.Exists(filepath) {
func getRunLog(cache RunLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run, attempt uint64) (*zip.ReadCloser, error) {
cacheKey := fmt.Sprintf("%d-%d", run.ID, run.StartedTime().Unix())
isCached, err := cache.Exists(cacheKey)
if err != nil {
return nil, err
}
if !isCached {
// Run log does not exist in cache so retrieve and store it
logURL := fmt.Sprintf("%srepos/%s/actions/runs/%d/logs",
ghinstance.RESTPrefix(repo.RepoHost()), ghrepo.FullName(repo), run.ID)
@ -461,13 +490,13 @@ func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface
return nil, err
}
err = cache.Create(filepath, respReader)
err = cache.Create(cacheKey, respReader)
if err != nil {
return nil, err
}
}
return cache.Open(filepath)
return cache.Open(cacheKey)
}
func promptForJob(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, error) {

21
pkg/cmd/run/view/view_test.go
View file

@ -12,6 +12,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/run/shared"
@ -137,6 +138,9 @@ func TestNewCmdView(t *testing.T) {
f := &cmdutil.Factory{
IOStreams: ios,
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
}
argv, err := shlex.Split(tt.cli)
@ -1342,8 +1346,9 @@ func TestViewRun(t *testing.T) {
browser := &browser.Stub{}
tt.opts.Browser = browser
rlc := testRunLogCache{}
tt.opts.RunLogCache = rlc
tt.opts.RunLogCache = RunLogCache{
cacheDir: t.TempDir(),
}
t.Run(tt.name, func(t *testing.T) {
err := runView(tt.opts)
@ -1503,18 +1508,6 @@ func Test_attachRunLog(t *testing.T) {
}
}
type testRunLogCache struct{}
func (testRunLogCache) Exists(path string) bool {
return false
}
func (testRunLogCache) Create(path string, content io.Reader) error {
return nil
}
func (testRunLogCache) Open(path string) (*zip.ReadCloser, error) {
return zip.OpenReader("./fixtures/run_log.zip")
}
var barfTheFobLogOutput = heredoc.Doc(`
cool job barz the fob log line 1
cool job barz the fob log line 2