Add support for GitHub Actions cache API by alidevhere · Pull Request #2604 · google/go-github

alidevhere · 2022-12-20T14:02:37Z

@gmlewis Ready for review. Please mention if something needs a change.
Thanks

Fixes: #2584.

google-cla · 2022-12-20T14:02:41Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

codecov · 2022-12-20T14:22:08Z

Codecov Report

Merging #2604 (7e6570e) into master (c5583e7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2604      +/-   ##
==========================================
+ Coverage   97.99%   98.01%   +0.01%     
==========================================
  Files         127      128       +1     
  Lines       10948    11034      +86     
==========================================
+ Hits        10729    10815      +86     
  Misses        150      150              
  Partials       69       69

Impacted Files	Coverage Δ
github/actions_cache.go	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

gmlewis

Thank you, @alidevhere !

gmlewis · 2022-12-20T14:24:28Z

github/actions_cache.go

+//
+// GitHub API docs: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28#list-github-actions-caches-for-a-repository
+type ActionsCacheList struct {
+	TotalCount    *int64          `json:"total_count,omitempty"`


I think this one is safe to leave as *int (not *int64 as we tend to reserve that for really large numbers or numeric IDs).

gmlewis · 2022-12-20T14:25:26Z

github/actions_cache.go

+//
+//GitHub API docs: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28#get-github-actions-cache-usage-for-a-repository
+type ActionsCacheUsage struct {
+	FullName             *string `json:"full_name"`


We tend to add omitempty when are not sure if the GitHub server will populate the field...
but if we are confident that this will be a populated field, then we can get rid of the pointer and make this a string instead of a *string.

gmlewis · 2022-12-20T14:26:11Z

github/actions_cache.go

+//GitHub API docs: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28#get-github-actions-cache-usage-for-a-repository
+type ActionsCacheUsage struct {
+	FullName             *string `json:"full_name"`
+	ActiveCacheUsageSize *int64  `json:"active_caches_size_in_bytes"`


We try to match the name of the field to avoid confusion, so this would be ActiveCachesSizeInBytes, and I think this one is fine to leave as an int64.

gmlewis · 2022-12-20T14:26:28Z

github/actions_cache.go

+type ActionsCacheUsage struct {
+	FullName             *string `json:"full_name"`
+	ActiveCacheUsageSize *int64  `json:"active_caches_size_in_bytes"`
+	ActiveCachesCount    *int64  `json:"active_caches_count"`


I'm thinking that this one should probably be an int.

gmlewis · 2022-12-20T14:26:55Z

github/actions_cache.go

+	ActiveCachesCount    *int64  `json:"active_caches_count"`
+}
+
+// ActionsCacheUsageList represents a list repositories with GitHub Actions cache usage for an organization.


"list repositories" => "list of repositories"

github/actions_cache.go

gmlewis · 2022-12-20T14:43:18Z

github/actions_cache.go

+	return cacheUsage, res, err
+}
+
+// Gets the total GitHub Actions cache usage for an enterprise. The data fetched using this API is refreshed approximately every 5 minutes,


Suggested change

// Gets the total GitHub Actions cache usage for an enterprise. The data fetched using this API is refreshed approximately every 5 minutes,

// GetTotalCacheUsageForEnterprise gets the total GitHub Actions cache usage for an enterprise. The data fetched using this API is refreshed approximately every 5 minutes,

github/actions_cache.go

gmlewis · 2022-12-20T14:43:56Z

github/actions_cache.go

+	u := fmt.Sprintf("enterprises/%v/actions/cache/usage", enterprise)
+
+	req, err := s.client.NewRequest("GET", u, nil)
+


Please delete this blank line. See above.

gmlewis · 2022-12-20T14:44:02Z

github/actions_cache.go

+
+	cacheUsage := new(TotalCacheUsage)
+	res, err := s.client.Do(ctx, req, cacheUsage)
+


Please delete this blank line. See above.

alidevhere · 2022-12-22T12:52:25Z

@gmlewis I have made all requested changes and re ran all tests and commands.

Please review again and do mention if something is still missing.
Thanks 😃

gmlewis

Thank you, @alidevhere !
Just a few more tweaks, please, then I think we will be ready for a second LGTM+Approval from any other contributor to this repo before merging.

gmlewis · 2022-12-22T19:21:44Z

github/actions_cache.go

+//
+// GitHub API docs: https://docs.github.com/en/rest/actions/cache?apiVersion=2022-11-28#get-github-actions-cache-usage-for-an-enterprise
+type TotalCacheUsage struct {
+	TotalActiveCacheUsageSizeInBytes int64 `json:"total_active_caches_size_in_bytes"`


Suggested change

TotalActiveCacheUsageSizeInBytes int64 `json:"total_active_caches_size_in_bytes"`

TotalActiveCachesUsageSizeInBytes int64 `json:"total_active_caches_size_in_bytes"`

gmlewis · 2022-12-22T19:23:13Z

github/actions_cache.go

+	Ref string `url:"ref,omitempty"`
+	Key string `url:"key,omitempty"`
+	// Can be one of: "created_at", "last_accessed_at", "size_in_bytes". Default: "last_accessed_at"
+	Sort string `url:"sort,omitempty"`
+	// Can be one of: "asc", "desc" Default: desc
+	Direction string `url:"direction,omitempty"`


Suggested change

Ref string `url:"ref,omitempty"`

Key string `url:"key,omitempty"`

// Can be one of: "created_at", "last_accessed_at", "size_in_bytes". Default: "last_accessed_at"

Sort string `url:"sort,omitempty"`

// Can be one of: "asc", "desc" Default: desc

Direction string `url:"direction,omitempty"`

Ref *string `url:"ref,omitempty"`

Key *string `url:"key,omitempty"`

// Can be one of: "created_at", "last_accessed_at", "size_in_bytes". Default: "last_accessed_at"

Sort *string `url:"sort,omitempty"`

// Can be one of: "asc", "desc" Default: desc

Direction *string `url:"direction,omitempty"`

gmlewis

Thank you, @alidevhere !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

valbeat

LGTM!

gmlewis · 2023-01-05T12:05:32Z

Thank you, @valbeat !
Merging.

gmlewis changed the title ~~Issue # 2584~~ Add support for GitHub Actions cache API Dec 20, 2022

gmlewis requested changes Dec 20, 2022

View reviewed changes

alidevhere requested a review from gmlewis December 22, 2022 12:48

gmlewis requested changes Dec 22, 2022

View reviewed changes

gmlewis added the NeedsReview PR is awaiting a review before merging. label Dec 22, 2022

Ali added 4 commits December 29, 2022 01:30

implemented github actions

b9439d0

changes

524b7f0

all tests passed

49f753e

asked changes done

7e6570e

alidevhere force-pushed the Issue-2584 branch from 8cf0e07 to 7e6570e Compare December 28, 2022 20:30

alidevhere requested a review from gmlewis December 28, 2022 20:32

gmlewis approved these changes Dec 28, 2022

View reviewed changes

valbeat approved these changes Jan 5, 2023

View reviewed changes

gmlewis removed the NeedsReview PR is awaiting a review before merging. label Jan 5, 2023

gmlewis merged commit 8ec1e49 into google:master Jan 5, 2023

	// Gets the total GitHub Actions cache usage for an enterprise. The data fetched using this API is refreshed approximately every 5 minutes,
	// GetTotalCacheUsageForEnterprise gets the total GitHub Actions cache usage for an enterprise. The data fetched using this API is refreshed approximately every 5 minutes,

		u := fmt.Sprintf("enterprises/%v/actions/cache/usage", enterprise)

		req, err := s.client.NewRequest("GET", u, nil)


		cacheUsage := new(TotalCacheUsage)
		res, err := s.client.Do(ctx, req, cacheUsage)

	TotalActiveCacheUsageSizeInBytes int64 `json:"total_active_caches_size_in_bytes"`
	TotalActiveCachesUsageSizeInBytes int64 `json:"total_active_caches_size_in_bytes"`

Conversation

alidevhere commented Dec 20, 2022 • edited by gmlewis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

google-cla bot commented Dec 20, 2022

Uh oh!

codecov bot commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alidevhere commented Dec 22, 2022

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

valbeat left a comment

Choose a reason for hiding this comment

Uh oh!

gmlewis commented Jan 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alidevhere commented Dec 20, 2022 •

edited by gmlewis

Loading

codecov bot commented Dec 20, 2022 •

edited

Loading