feat: Add support for conditional requests via local cache (#17368)

erezrokah · web-flow · commit 6d866d18f3e8 · 2024-04-02T22:34:19.000+03:00
#### Summary Implements https://github.com/google/go-github?tab=readme-ov-file#conditional-requests Tested it and indeed saw some requests returned from the cache and not counting towards the limit
diff --git a/plugins/source/github/client/client.go b/plugins/source/github/client/client.go
@@ -10,6 +10,8 @@ import (
 	"time"
 
 	"github.com/bradleyfalzon/ghinstallation/v2"
+	"github.com/cloudquery/httpcache"
+	"github.com/cloudquery/httpcache/diskcache"
 	"github.com/cloudquery/plugin-sdk/v4/schema"
 	"github.com/gofri/go-github-ratelimit/github_ratelimit"
 	"github.com/google/go-github/v59/github"
@@ -91,10 +93,15 @@ func New(ctx context.Context, logger zerolog.Logger, spec Spec) (schema.ClientMe
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse InstallationID %v: %w", auth.InstallationID, err)
 		}
+		transport := http.DefaultTransport
+		if spec.LocalCachePath != "" {
+			cache := diskcache.New(spec.LocalCachePath)
+			transport = httpcache.NewTransport(cache)
+		}
 		if auth.PrivateKeyPath != "" {
-			itr, err = ghinstallation.NewKeyFromFile(http.DefaultTransport, appId, installationId, auth.PrivateKeyPath)
+			itr, err = ghinstallation.NewKeyFromFile(transport, appId, installationId, auth.PrivateKeyPath)
 		} else {
-			itr, err = ghinstallation.New(http.DefaultTransport, appId, installationId, []byte(auth.PrivateKey))
+			itr, err = ghinstallation.New(transport, appId, installationId, []byte(auth.PrivateKey))
 		}
 		if err != nil {
 			return nil, fmt.Errorf("failed to create GitHub client for org %v: %w", auth.Org, err)
@@ -111,7 +118,12 @@ func New(ctx context.Context, logger zerolog.Logger, spec Spec) (schema.ClientMe
 
 	var defaultServices GithubServices
 	if spec.AccessToken != "" {
-		httpClient := github.NewClient(nil).WithAuthToken(spec.AccessToken)
+		var cl *http.Client
+		if spec.LocalCachePath != "" {
+			cache := diskcache.New(spec.LocalCachePath)
+			cl = httpcache.NewTransport(cache).Client()
+		}
+		httpClient := github.NewClient(cl).WithAuthToken(spec.AccessToken)
 		ghc, err := githubClientForHTTPClient(httpClient.Client().Transport, logger, spec.EnterpriseSettings)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create GitHub client for access token: %w", err)
diff --git a/plugins/source/github/client/schema.json b/plugins/source/github/client/schema.json
diff --git a/plugins/source/github/client/spec.go b/plugins/source/github/client/spec.go
@@ -3,6 +3,7 @@ package client
 import (
 	_ "embed"
 	"fmt"
+	"os"
 
 	"github.com/invopop/jsonschema"
 )
@@ -25,6 +26,8 @@ type Spec struct {
 	DiscoveryConcurrency int `json:"discovery_concurrency,omitempty" jsonschema:"default=1"`
 	// Include archived repositories when discovering repositories.
 	IncludeArchivedRepos bool `json:"include_archived_repos,omitempty"`
+	// Path to a local directory that will hold the cache. If set, the plugin will cache the GitHub API responses in this directory. Defaults to an empty string (no cache)
+	LocalCachePath string `json:"local_cache_path,omitempty"`
 }
 
 type EnterpriseSettings struct {
@@ -87,6 +90,15 @@ func (s *Spec) Validate() error {
 			return err
 		}
 	}
+	if s.LocalCachePath != "" {
+		fileInfo, err := os.Stat(s.LocalCachePath)
+		if err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("error accessing local cache path: %w", err)
+		}
+		if fileInfo != nil && !fileInfo.IsDir() {
+			return fmt.Errorf("local cache path is not a directory")
+		}
+	}
 	return nil
 }
 
diff --git a/plugins/source/github/docs/_configuration.md b/plugins/source/github/docs/_configuration.md
@@ -38,6 +38,7 @@ spec:
     # concurrency: 1500 # Optional. The best effort maximum number of Go routines to use. Lower this number to reduce memory usage or to avoid hitting GitHub API rate limits. Default 1500.
     # discovery_concurrency: 1 # Optional. Number of concurrent requests to GitHub API during discovery phase. Default 1.
     # include_archived_repos: false # Optional. Include archived repositories in the sync. Default false.
+    # local_cache_path: "" # Optional. Path to a local directory that will hold the cache. If set, the plugin will cache the GitHub API responses in this directory. Defaults to an empty string (no cache).
 ```
 
 See [tables](/docs/plugins/sources/github/tables) for a full list of available tables.
diff --git a/plugins/source/github/docs/overview.md b/plugins/source/github/docs/overview.md
@@ -41,3 +41,7 @@ This is the (nested) spec used by GitHub Source Plugin
 - `include_archived_repos` (`bool`) (default: `false`)
 
   By default archived repositories are not included in the sync. To include archived repositories set `include_archived_repos` to `true`.
+
+- `local_cache_path` (`string`, optional, default: empty):
+  Path to a local directory that will hold the cache. If set, the plugin will cache the GitHub API responses in this directory. Defaults to an empty string (no cache).
+  By using a cache, the plugin can use [conditional requests when appropriate](https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?#use-conditional-requests-if-appropriate), and help avoid hitting GitHub API rate limits.
diff --git a/plugins/source/github/go.mod b/plugins/source/github/go.mod
@@ -6,6 +6,7 @@ require (
 	github.com/apache/arrow/go/v15 v15.0.0-20240114144300-7e703aae55c1
 	github.com/bradleyfalzon/ghinstallation/v2 v2.9.0
 	github.com/cloudquery/codegen v0.3.13
+	github.com/cloudquery/httpcache v0.0.0-20240402185306-9cae36e50fd1
 	github.com/cloudquery/plugin-sdk/v4 v4.36.3
 	github.com/ghodss/yaml v1.0.0
 	github.com/gofri/go-github-ratelimit v1.0.3
@@ -54,6 +55,7 @@ require (
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/golang/snappy v0.0.4 // indirect
 	github.com/gomarkdown/markdown v0.0.0-20231222211730-1d6d20845b47 // indirect
+	github.com/google/btree v1.1.2 // indirect
 	github.com/google/flatbuffers v23.5.26+incompatible // indirect
 	github.com/google/go-github/v57 v57.0.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
@@ -86,6 +88,7 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/pelletier/go-toml/v2 v2.1.1 // indirect
+	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/pierrec/lz4/v4 v4.1.21 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
diff --git a/plugins/source/github/go.sum b/plugins/source/github/go.sum
@@ -51,6 +51,8 @@ github.com/cloudquery/cloudquery-api-go v1.9.0 h1:yjy/Hrma6/1JdtGYE59xMtH+xAsaGL
 github.com/cloudquery/cloudquery-api-go v1.9.0/go.mod h1:F4kuaNBAVqsS9ZRHuX+tV2m6+Khoa2Rb9lROGhinGPk=
 github.com/cloudquery/codegen v0.3.13 h1:ZQZhaN/xXyr8qBhIlyPuTyncssI8JqsKwIQamWxIAPM=
 github.com/cloudquery/codegen v0.3.13/go.mod h1:gF9EOua45yxt5n5Zc8NrZ63HT6rGc6wDCV+Apc9d0Mg=
+github.com/cloudquery/httpcache v0.0.0-20240402185306-9cae36e50fd1 h1:KFXFLIF3t1KPAZJe1TmM/ro9T1PfD+hkuLonCurwwcU=
+github.com/cloudquery/httpcache v0.0.0-20240402185306-9cae36e50fd1/go.mod h1:0wYLAQ8f+qoGzvTQghj43oLHKmov4NtURVrW1pLI9kA=
 github.com/cloudquery/jsonschema v0.0.0-20240220124159-92878faa2a66 h1:OZLPSIBYEfvkAUeOeM8CwTgVQy5zhayI99ishCrsFV0=
 github.com/cloudquery/jsonschema v0.0.0-20240220124159-92878faa2a66/go.mod h1:0SoZ/U7yJlNOR+fWsBSeTvTbGXB6DK01tzJ7m2Xfg34=
 github.com/cloudquery/plugin-pb-go v1.19.7 h1:9AfnIIFRFTuo0CMP0SCbMGWfDQ0I6C9m/jrglWcqDzA=
@@ -115,6 +117,8 @@ github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
 github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
 github.com/gomarkdown/markdown v0.0.0-20231222211730-1d6d20845b47 h1:k4Tw0nt6lwro3Uin8eqoET7MDA4JnT8YgbCjc/g5E3k=
 github.com/gomarkdown/markdown v0.0.0-20231222211730-1d6d20845b47/go.mod h1:JDGcbDT52eL4fju3sZ4TeHGsQwhG9nbDV21aMyhwPoA=
+github.com/google/btree v1.1.2 h1:xf4v41cLI2Z6FxbKm+8Bu+m8ifhj15JuZ9sa0jZCMUU=
+github.com/google/btree v1.1.2/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4=
 github.com/google/flatbuffers v23.5.26+incompatible h1:M9dgRyhJemaM4Sw8+66GHBu8ioaQmyPLg1b8VwK5WJg=
 github.com/google/flatbuffers v23.5.26+incompatible/go.mod h1:1AeVuKshWv4vARoZatz6mlQ0JxURH0Kv5+zNeJKJCa8=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -209,6 +213,8 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/pelletier/go-toml/v2 v2.1.1 h1:LWAJwfNvjQZCFIDKWYQaM62NcYeYViCmWIwmOStowAI=
 github.com/pelletier/go-toml/v2 v2.1.1/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
+github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
+github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/pierrec/lz4/v4 v4.1.21 h1:yOVMLb6qSIDP67pl/5F7RepeKYu/VmTyEXvuMI5d9mQ=
 github.com/pierrec/lz4/v4 v4.1.21/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
 github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ package client`
`3`	`3`	`import (`
`4`	`4`	`_ "embed"`
`5`	`5`	`"fmt"`
	`6`	`+ "os"`
`6`	`7`
`7`	`8`	`"github.com/invopop/jsonschema"`
`8`	`9`	`)`
`@@ -25,6 +26,8 @@ type Spec struct {`
`25`	`26`	DiscoveryConcurrency int `json:"discovery_concurrency,omitempty" jsonschema:"default=1"`
`26`	`27`	`// Include archived repositories when discovering repositories.`
`27`	`28`	IncludeArchivedRepos bool `json:"include_archived_repos,omitempty"`
	`29`	`+ // Path to a local directory that will hold the cache. If set, the plugin will cache the GitHub API responses in this directory. Defaults to an empty string (no cache)`
	`30`	+ LocalCachePath string `json:"local_cache_path,omitempty"`
`28`	`31`	`}`
`29`	`32`
`30`	`33`	`type EnterpriseSettings struct {`
`@@ -87,6 +90,15 @@ func (s *Spec) Validate() error {`
`87`	`90`	`return err`
`88`	`91`	`}`
`89`	`92`	`}`
	`93`	`+ if s.LocalCachePath != "" {`
	`94`	`+ fileInfo, err := os.Stat(s.LocalCachePath)`
	`95`	`+ if err != nil && !os.IsNotExist(err) {`
	`96`	`+ return fmt.Errorf("error accessing local cache path: %w", err)`
	`97`	`+ }`
	`98`	`+ if fileInfo != nil && !fileInfo.IsDir() {`
	`99`	`+ return fmt.Errorf("local cache path is not a directory")`
	`100`	`+ }`
	`101`	`+ }`
`90`	`102`	`return nil`
`91`	`103`	`}`
`92`	`104`