Skip to content

Commit 389e3af

Browse files
committed
fix: override should_build function in noop_builder
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
1 parent eb5a67f commit 389e3af

3 files changed

Lines changed: 64 additions & 0 deletions

File tree

flytekit/image_spec/noop_builder.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@ class NoOpBuilder(ImageSpecBuilder):
66

77
builder_type = "noop"
88

9+
def should_build(self, image_spec: ImageSpec) -> bool:
10+
"""
11+
The build_image function of NoOpBuilder uses the image_spec name as defined by the user without
12+
checking whether the image exists in the Docker registry. Therefore, the should_build function
13+
should always return True to trigger the build_image function.
14+
15+
Args:
16+
image_spec (ImageSpec): Image specification
17+
18+
Returns:
19+
bool: Always returns True
20+
"""
21+
return True
22+
923
def build_image(self, image_spec: ImageSpec) -> str:
1024
if not isinstance(image_spec.base_image, str):
1125
msg = "base_image must be a string to use the noop image builder"

tests/flytekit/unit/core/image_spec/test_image_spec.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,3 +359,37 @@ def test_with_builder_options():
359359
"existing_builder_option_1": "existing_builder_option_value_1",
360360
"new_builder_option_1": "new_builder_option_value_1"
361361
}
362+
363+
def test_noop_builder_updates_image_name_mapping():
364+
from flytekit.image_spec.noop_builder import NoOpBuilder
365+
366+
# Clear any existing mappings to ensure clean test state
367+
ImageBuildEngine._IMAGE_NAME_TO_REAL_NAME.clear()
368+
369+
# Register NoOpBuilder
370+
ImageBuildEngine.register("noop", NoOpBuilder())
371+
372+
# Create an image spec with NoOpBuilder
373+
expected_real_name = "localhost:30000/test_image:latest"
374+
image_spec = ImageSpec(
375+
name="test_image",
376+
builder="noop",
377+
base_image=expected_real_name
378+
)
379+
380+
# Get the image name before building
381+
img_name = image_spec.image_name()
382+
383+
# Build the image
384+
ImageBuildEngine.build(image_spec)
385+
386+
# Verify that the mapping was created in _IMAGE_NAME_TO_REAL_NAME
387+
assert img_name in ImageBuildEngine._IMAGE_NAME_TO_REAL_NAME
388+
389+
# Verify the mapping value is correct
390+
actual_real_name = ImageBuildEngine._IMAGE_NAME_TO_REAL_NAME[img_name]
391+
assert actual_real_name == expected_real_name
392+
393+
# Clean up
394+
del ImageBuildEngine._REGISTRY["noop"]
395+
ImageBuildEngine._IMAGE_NAME_TO_REAL_NAME.clear()

tests/flytekit/unit/core/image_spec/test_noop_builder.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,19 @@ def test_noop_builder_error(base_image):
2222
image_spec = ImageSpec(base_image=base_image)
2323
with pytest.raises(ValueError, match=msg):
2424
builder.build_image(image_spec)
25+
26+
27+
def test_noop_builder_should_build():
28+
"""Test that NoOpBuilder.should_build always returns True."""
29+
builder = NoOpBuilder()
30+
31+
# Test with different image specs to ensure should_build always returns True
32+
test_cases = [
33+
ImageSpec(base_image="localhost:30000/flytekit"),
34+
ImageSpec(base_image="python:3.9"),
35+
ImageSpec(base_image="custom/image:latest"),
36+
]
37+
38+
for image_spec in test_cases:
39+
result = builder.should_build(image_spec)
40+
assert result is True, f"should_build should return True for {image_spec.base_image}"

0 commit comments

Comments
 (0)