From d7128078c1165ec14738a364c1886b3ea1e95047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Thu, 6 Mar 2025 15:26:47 +0100 Subject: [PATCH 1/4] cli: rely on the existing use_s3_caching option to enable caching --- lib/omnibus/cli.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/omnibus/cli.rb b/lib/omnibus/cli.rb index e228079b3..99c1d0828 100644 --- a/lib/omnibus/cli.rb +++ b/lib/omnibus/cli.rb @@ -85,8 +85,8 @@ def build(name) project = Project.load(name, manifest) say("Building #{project.name} #{project.build_version}...") # Not sure why we need the !Omnibus::S3Cache.fetch_missing.empty? check here - Omnibus::S3Cache.populate if @options[:populate_s3_cache] && !Omnibus::S3Cache.fetch_missing.empty? - Omnibus::S3LicenseCache.populate if @options[:populate_s3_cache] + Omnibus::S3Cache.populate if Omnibus::Config.use_s3_caching && !Omnibus::S3Cache.fetch_missing.empty? + Omnibus::S3LicenseCache.populate if Omnibus::Config.use_s3_caching project.download project.build From 904d7d2018acdc684e25f661255dacd4454ff2f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Thu, 6 Mar 2025 17:13:57 +0100 Subject: [PATCH 2/4] cli: remove now unused populate_s3_cache option --- lib/omnibus/cli.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/omnibus/cli.rb b/lib/omnibus/cli.rb index 99c1d0828..b6a65c481 100644 --- a/lib/omnibus/cli.rb +++ b/lib/omnibus/cli.rb @@ -70,10 +70,6 @@ def execute! desc: "Use the given manifest when downloading software sources.", type: :string, default: nil - method_option :populate_s3_cache, - desc: "Populate the S3 cache.", - type: :boolean, - default: false desc "build PROJECT", "Build the given Omnibus project" def build(name) manifest = if @options[:use_manifest] From 80dabb6a4712e6218151224eeb389774225d5386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Fri, 7 Mar 2025 08:16:13 +0100 Subject: [PATCH 3/4] don't attempt to populate cache when we only have read access --- lib/omnibus/cli.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/omnibus/cli.rb b/lib/omnibus/cli.rb index b6a65c481..81c689fe3 100644 --- a/lib/omnibus/cli.rb +++ b/lib/omnibus/cli.rb @@ -81,8 +81,8 @@ def build(name) project = Project.load(name, manifest) say("Building #{project.name} #{project.build_version}...") # Not sure why we need the !Omnibus::S3Cache.fetch_missing.empty? check here - Omnibus::S3Cache.populate if Omnibus::Config.use_s3_caching && !Omnibus::S3Cache.fetch_missing.empty? - Omnibus::S3LicenseCache.populate if Omnibus::Config.use_s3_caching + Omnibus::S3Cache.populate if Omnibus::Config.use_s3_caching && Omnibus::Config.s3_authenticated_download && !Omnibus::S3Cache.fetch_missing.empty? + Omnibus::S3LicenseCache.populate if Omnibus::Config.use_s3_caching && Omnibus::Config.s3_authenticated_download project.download project.build From 1386d59b0c22cbe026bc6cf99625e15a68f01874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Fri, 7 Mar 2025 12:06:48 +0100 Subject: [PATCH 4/4] add a comment to clarify the logic --- lib/omnibus/cli.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/omnibus/cli.rb b/lib/omnibus/cli.rb index 81c689fe3..79a4d196c 100644 --- a/lib/omnibus/cli.rb +++ b/lib/omnibus/cli.rb @@ -81,6 +81,9 @@ def build(name) project = Project.load(name, manifest) say("Building #{project.name} #{project.build_version}...") # Not sure why we need the !Omnibus::S3Cache.fetch_missing.empty? check here + # We only want to populate the cache when we actually can. In some cases, we're only reading anonymously + # from the cache bucket (mostly when the build isn't ran on gitlab). When that is the case, we want to disable + # the cache population since it would fail. Omnibus::S3Cache.populate if Omnibus::Config.use_s3_caching && Omnibus::Config.s3_authenticated_download && !Omnibus::S3Cache.fetch_missing.empty? Omnibus::S3LicenseCache.populate if Omnibus::Config.use_s3_caching && Omnibus::Config.s3_authenticated_download project.download