Skip to content

Commit 09dbb73

Browse files
committed
style: Address feedback from design review
Responds to code review comments by: - Reformatting all function docstrings to meet the style guide. - Removing stale and redundant comments. - Moving local run instructions to the module docstring for better visibility. - Adding a clarifying comment for the SUPPORTED_VERSIONS variable.
1 parent 79779ba commit 09dbb73

File tree

2 files changed

+30
-25
lines changed

2 files changed

+30
-25
lines changed

infra/build/functions/base_images.py

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,18 @@
1010
# distributed under the License is distributed on an "AS IS" BASIS,
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
13-
# limitations under the License.n#
13+
# limitations under the License.
14+
#
1415
################################################################################
15-
"""Cloud function to build base images on Google Cloud Builder."""
16+
"""Cloud function to build base images on Google Cloud Builder.
17+
18+
This script can be run locally for testing or deployment purposes. By default,
19+
it performs a real build. To perform a dry run, use the '--dry-run' flag. To
20+
prevent images from being pushed to the registry, use '--no-push'.
21+
22+
Example:
23+
python3 infra/build/functions/base_images.py --dry-run
24+
"""
1625
from collections.abc import Sequence
1726
import logging
1827
import os
@@ -27,8 +36,11 @@
2736
MAJOR_TAG = 'v1'
2837
TIMEOUT = '21600' # 6 hours
2938

30-
# Define the supported Ubuntu versions for base images.
39+
# Defines the Ubuntu versions supported by the build infrastructure.
3140
# 'legacy' refers to the unversioned, default image.
41+
# Note: This list indicates build capability, not production readiness.
42+
# A version is only ready for general use after being fully enabled in
43+
# ClusterFuzz.
3244
SUPPORTED_VERSIONS = ('legacy', 'ubuntu-20-04', 'ubuntu-24-04')
3345

3446
# Define which of the supported versions is considered the default.
@@ -62,9 +74,10 @@ def _get_default_path(self) -> str:
6274
return os.path.join('infra', 'base-images', self.name)
6375

6476
def _resolve_dockerfile(self) -> str:
65-
"""
66-
Resolves the path to the Dockerfile, preferring a version-specific
67-
one if it exists, otherwise falling back to the legacy Dockerfile.
77+
"""Resolves the path to the Dockerfile.
78+
79+
Prefers a version-specific one if it exists, otherwise falling back to the
80+
legacy Dockerfile.
6881
"""
6982
if self.version != 'legacy':
7083
versioned_dockerfile = os.path.join(self.path,
@@ -168,10 +181,10 @@ def run_build(steps: list[dict],
168181
tags: list[str] | None = None,
169182
dry_run: bool = False,
170183
no_push: bool = False):
184+
"""Executes a build in GCB and pushes the resulting images.
185+
186+
Alternatively, prints the configuration if in dry_run mode.
171187
"""
172-
Executes a build in GCB and pushes the resulting images, or prints the
173-
configuration if in dry_run mode.
174-
"""
175188
if dry_run:
176189
print(
177190
'--------------------------------------------------------------------')
@@ -224,10 +237,11 @@ def run_build(steps: list[dict],
224237

225238

226239
def get_images_architecture_manifest_steps(target_tag: str) -> list[dict]:
240+
"""Returns steps for creating and pushing a multi-architecture manifest.
241+
242+
The manifest is for the base-builder and base-runner images with a
243+
specific tag.
227244
"""
228-
Returns steps to create and push a multi-architecture manifest for
229-
the base-builder and base-runner images with a specific tag.
230-
"""
231245
images = [
232246
f'{IMAGE_NAME_PREFIX}base-builder', f'{IMAGE_NAME_PREFIX}base-runner'
233247
]
@@ -239,10 +253,7 @@ def get_images_architecture_manifest_steps(target_tag: str) -> list[dict]:
239253

240254
def get_image_push_architecture_manifest_steps(image: str,
241255
target_tag: str) -> list[dict]:
242-
"""
243-
Returns the steps to push a manifest pointing to ARM64 and AMD64
244-
versions of an |image| with a specific |target_tag|.
245-
"""
256+
"""Returns steps for pushing a manifest pointing to ARM64/AMD64 versions."""
246257
# The AMD64 image is the one we just built.
247258
amd64_source_image = f'{image}:{target_tag}'
248259
# The ARM64 image is a pre-built generic testing image.
@@ -295,10 +306,10 @@ def get_image_push_architecture_manifest_steps(image: str,
295306

296307

297308
def base_builder(event, context, dry_run: bool = False, no_push: bool = False):
309+
"""Cloud function entry point.
310+
311+
Triggers parallel base image builds for each supported Ubuntu version.
298312
"""
299-
Cloud function entry point. Triggers parallel base image builds for each
300-
supported Ubuntu version.
301-
"""
302313
del event, context
303314
logging.basicConfig(level=logging.INFO)
304315

@@ -332,10 +343,6 @@ def base_builder(event, context, dry_run: bool = False, no_push: bool = False):
332343

333344

334345
if __name__ == '__main__':
335-
# This allows the script to be run locally for testing or deployment.
336-
# By default, it performs a real build.
337-
# To perform a dry run, pass the '--dry-run' flag.
338-
# To prevent pushing images to the registry, pass '--no-push'.
339346
is_dry_run = '--dry-run' in sys.argv
340347
no_push = '--no-push' in sys.argv
341348
base_builder(None, None, dry_run=is_dry_run, no_push=no_push)

infra/build/functions/report_generator.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ def main():
147147
any_results_found = True
148148
if data.get('failed_builds', 0) > 0:
149149
any_failures = True
150-
# Use .get() for safe access to prevent potential KeyErrors.
151150
total_unique_projects.update(data.get('all_projects', []))
152151

153152
if not any_results_found:
@@ -162,7 +161,6 @@ def main():
162161
generate_comparison_table(all_results)
163162
generate_final_summary(all_results)
164163

165-
# Define failure conditions explicitly for clarity.
166164
has_explicit_failures = any_failures
167165
no_projects_were_run = any_results_found and not total_unique_projects
168166

0 commit comments

Comments
 (0)