Skip to content
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

Added TRANSFROM_NAME to docker build arg #929

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

matouma
Copy link
Contributor

@matouma matouma commented Jan 8, 2025

…f docker file template

Why are these changes needed?

While doing the refactoring of the transforms, we observed that now, most of the Dockerfile have the same scripts except for the module name. This change adds TRANSFORM_NAME build arg and dockerfile templates for python, ray and spark so transforms owner can simply copy/paste the template. In most cases, the transform owner will not need to modify the file unless their transform require additional non-common components that are unique to the run time environment for this transform

Related issue number (if any).

[Feature] Restructure transforms as their own modules #774

@touma-I touma-I requested review from daw3rd, roytman and revit13 January 8, 2025 23:38

# END OF STEPS destined for a data-prep-kit base image

COPY --chown=dpk:root dpk_${TRANSFORM_NAME}/ dpk_${TRANSFORM_NAME}/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be --chown=dpk:users

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks

# Install project source

## Copy the python version of the tansform
COPY --chown=spark:root dpk_${TRANSFORM_NAME}/ dpk_${TRANSFORM_NAME}/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above --chown=spark:root

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks

@@ -0,0 +1,31 @@
ARG BASE_IMAGE=docker.io/rayproject/ray:2.24.0-py310
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use this BASE_IMAGE mechanism with Ray only, should it be extended to Python and Spark as well?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roytman I'd like to defer this to another PR when I do the full cleanup of the Makefile and other CI/CD related artifacts. #930 For this PR, I just want to setup the templates so developers of new transforms don't have to edit the Dockerfiles and simply use the template as is unless they have some complex custom requirements. I will need to do another round on the templates when I do the Makefile here #930

Signed-off-by: Maroun Touma <[email protected]>
@touma-I touma-I marked this pull request as ready for review January 9, 2025 13:02
@touma-I touma-I merged commit 9eab8b8 into IBM:dev Jan 9, 2025
137 checks passed
@touma-I touma-I deleted the Dockerfile-template branch January 9, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants