Remove DOCKER_REMOTE_IMAGE from .make.defaults #890
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
touma-I
changed the title
|
LGTM please remove the assignment of |
revit13
reviewed
Dec 18, 2024
transforms/.make.transforms
Outdated
| @@ -35,6 +35,10 @@ DOCKER_IMAGE_NAME?=$(TRANSFORM_NAME)-$(TRANSFORM_RUNTIME) | |||
| TRANSFORM_RUNTIME_SRC_FILE?=$(TRANSFORM_NAME)_transform_$(TRANSFORM_RUNTIME).py | |||
| TRANSFORM_TEST_FILE?=test/test_$(TRANSFORM_NAME).py | |||
|
|
|||
| ## Used when publish or building the image by legacy tansforms. | |||
| ## Will be removed when all transfdorms get refactored | |||
There was a problem hiding this comment.
oops. Done. Thanks
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Signed-off-by: Maroun Touma <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
In recent transform refactoring PRs, such as those for #878 and #869, the kind load command is executed twice for the transform image in the kfp tests. This happens instead of executing it once for the kfp-data-processing image and once for the transform image. This issue can be observed in the logs of KFP runs in these PRs by searching for the kind load string.
This PR aims to address this issue.
Explanation of the Error:
To understand why kfp-data-processing:latest quay.io/dataprep1/data-prep-kit/kfp-data-processing:latest is not loaded as expected in kfp-load-kind target let's look at the Makefile in kfp/kfp_ray_components:
in its first line it includes .make.defaults which defines DOCKER_REMOTE_IMAGE:
DOCKER_REMOTE_IMAGE=$(DOCKER_REGISTRY_ENDPOINT)/$(DOCKER_IMAGE_NAME):$(DOCKER_IMAGE_VERSION)
This is a recursively expanded variable, meaning its value is dynamically recalculated each time it is referenced, using the current values of any variables it depends on.
The recent transform refactoring PRs define the DOCKER_IMAGE_NAME variable in kfp_ray/Makefile as follows:
DOCKER_IMAGE_NAME =$(TRANSFORM_NAME)-$ (TRANSFORM_RUNTIME)
Which is also recursively expanded variable, causing DOCKER_REMOTE_IMAGE to be as follows:
DOCKER_REMOTE_IMAGE=$(DOCKER_REGISTRY_ENDPOINT)/$(TRANSFORM_NAME)-$(TRANSFORM_RUNTIME):$(DOCKER_IMAGE_VERSION)
This explains why when executing kind-load-image target DOCKER_REMOTE_IMAGE equals to quay.io/dataprep1/data-prep-kit/ededup-ray:latest.
Related issue number (if any).