-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Cache API verification results across CI runs #5042
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fa0901c
ba1b294
083835c
cd2fa52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,3 +8,4 @@ vendor | |
| .bundle | ||
| .idea | ||
| .tool-versions | ||
| .api-cache.json | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,6 +270,89 @@ def add_message(type, file, line_number, message) | |
| client.messages << "::#{type} file=#{file},line=#{line_number}::#{message}" | ||
| end | ||
|
|
||
| CACHE_FILE = File.expand_path("../.api-cache.json", __dir__) | ||
| CACHE_TTL_SECONDS = 24 * 60 * 60 # 24 hours | ||
|
|
||
| def load_api_cache! | ||
| return unless File.exist?(CACHE_FILE) | ||
|
|
||
| data = JSON.parse(File.read(CACHE_FILE)) | ||
| now = Time.now.to_i | ||
| ttl = CACHE_TTL_SECONDS | ||
|
|
||
| if data["repos"] | ||
| data["repos"].each do |key, entry| | ||
| cached_at = entry["cached_at"] | ||
| next unless cached_at | ||
| next if now - cached_at.to_i > ttl | ||
|
|
||
| result = entry["value"] | ||
| # Reconstruct a minimal object that responds to .full_name | ||
| cached = if result.nil? | ||
| nil | ||
| else | ||
| next unless result["full_name"] | ||
|
|
||
| Struct.new(:full_name).new(result["full_name"]) | ||
| end | ||
| NewOctokit.class_variable_get(:@@repos)[key] = cached | ||
| end | ||
| end | ||
|
|
||
| if data["users"] | ||
| data["users"].each do |key, entry| | ||
| cached_at = entry["cached_at"] | ||
| next unless cached_at | ||
| next if now - cached_at.to_i > ttl | ||
|
|
||
| result = entry["value"] | ||
| cached = if result.nil? | ||
| nil | ||
| else | ||
| next unless result["login"] | ||
|
|
||
| Struct.new(:login).new(result["login"]) | ||
| end | ||
| NewOctokit.class_variable_get(:@@users)[key] = cached | ||
| end | ||
| end | ||
| rescue JSON::ParserError, StandardError => e | ||
| warn "Failed to load API cache: #{e.message}" | ||
| end | ||
|
|
||
| def save_api_cache! | ||
| now = Time.now.to_i | ||
| repos_data = {} | ||
| users_data = {} | ||
|
|
||
| NewOctokit.class_variable_get(:@@repos).each do |key, value| | ||
| next if key == :skip_requests | ||
| next if value == true | ||
|
|
||
| repos_data[key.to_s] = { | ||
| "cached_at" => now, | ||
| "value" => value.nil? ? nil : { "full_name" => value.respond_to?(:full_name) ? value.full_name : value.to_s }, | ||
|
Comment on lines
+332
to
+334
|
||
| } | ||
| end | ||
|
|
||
| NewOctokit.class_variable_get(:@@users).each do |key, value| | ||
| next if key == :skip_requests | ||
| next if value == true | ||
|
|
||
| users_data[key.to_s] = { | ||
| "cached_at" => now, | ||
| "value" => value.nil? ? nil : { "login" => value.respond_to?(:login) ? value.login : value.to_s }, | ||
| } | ||
| end | ||
|
|
||
| File.write(CACHE_FILE, JSON.pretty_generate({ "repos" => repos_data, "users" => users_data })) | ||
| rescue StandardError => e | ||
| warn "Failed to save API cache: #{e.message}" | ||
| end | ||
|
|
||
| # Load cached API results at startup | ||
| load_api_cache! | ||
|
|
||
| Minitest.after_run do | ||
| warn "Repo checks were rate limited during this CI run" if NewOctokit.repos_skipped? | ||
| warn "User checks were rate limited during this CI run" if NewOctokit.users_skipped? | ||
|
|
@@ -279,4 +362,7 @@ def add_message(type, file, line_number, message) | |
| NewOctokit.messages.each do |message| | ||
| puts message | ||
| end | ||
|
|
||
| # Persist cache for next CI run | ||
| save_api_cache! | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a cache entry's
valueis not nil but also doesn't have afull_namekey (e.g., corrupted data), the code on line 292 will call.new(nil)sinceresult["full_name"]will be nil. This creates a struct withfull_name = nil, which might not match the expected behavior. Consider validating that required keys exist in the cached value before reconstructing the struct.This issue also appears on line 303 of the same file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed — added
next unless result["full_name"]andnext unless result["login"]validation before reconstructing the Struct, so corrupted/incomplete cache entries are skipped instead of creatingStruct.new(...).new(nil).