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

Move transform version numbers out of .make.versions to transform-specific file #657

Merged
merged 17 commits into from
Oct 9, 2024

Conversation

daw3rd
Copy link
Member

@daw3rd daw3rd commented Oct 2, 2024

Summary of changes

  1. For each transform create a transform.config file used for inclusion across the transform's Makefiles (for example, universal/noop/transform.config)
  2. Include this file in transform's Makefile (python/Makefile, ray/Makefile,spark/Makefile,kfp_ray/Makefile)
  3. Remove TRANSFORM_NAME from Makefiles (it is now in transform.config)
  4. Remove/comment out transform version numbers from .make.versions
  5. Expose USE_REPO_LIB_SRC macro in transform Makefiles to help developers switch to using pypi instead of rep source for dpk dependencies.
  6. Fix a problem in .make.defaults so that it does not update the dpk dependencies in a transform's toml when USE_REPO_LIB_SRC=0

Why are these changes needed?

Per issue #639 we would like to allow transforms to update their own version numbers.
However we don't want such a change to trigger ci/cd on all other transforms.
As such we need to move the version number out of .make.versions to a file that is more local to the transform.

Note that this is only the first of the TODOs listed in issue #639, but can be done independent of the others.

Related issue number (if any).

#639

@daw3rd daw3rd marked this pull request as draft October 2, 2024 18:21
@daw3rd daw3rd requested review from revit13, roytman and touma-I October 2, 2024 18:22
@daw3rd daw3rd marked this pull request as ready for review October 4, 2024 21:14
#
# If you change the versions numbers, be sure to run "make set-versions" to
# update version numbers across the transform (e.g., pyproject.toml).
CODE2PARQUET_PYTHON_VERSION=$(DPK_VERSION)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@daw3rd How are we getting DPK_VERSION if we are not including .make.versions ?

Copy link
Member

Choose a reason for hiding this comment

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

The transformer Makefiles include $(REPOROOT)/transforms/.make.transforms it includes $(REPOROOT)/.make.defaults and the last one includes $(REPOROOT)/.make.versions

@revit13
Copy link
Collaborator

revit13 commented Oct 9, 2024

@daw3rd scripts/transforms/update_workflow_tags.sh needs to be update to take the versions from the config files.

Copy link
Member

@roytman roytman left a comment

Choose a reason for hiding this comment

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

LGTM

#
# If you change the versions numbers, be sure to run "make set-versions" to
# update version numbers across the transform (e.g., pyproject.toml).
CODE2PARQUET_PYTHON_VERSION=$(DPK_VERSION)
Copy link
Member

Choose a reason for hiding this comment

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

The transformer Makefiles include $(REPOROOT)/transforms/.make.transforms it includes $(REPOROOT)/.make.defaults and the last one includes $(REPOROOT)/.make.versions

Copy link
Collaborator

@touma-I touma-I left a comment

Choose a reason for hiding this comment

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

Million thanks

@daw3rd daw3rd merged commit efc1162 into dev Oct 9, 2024
109 checks passed
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.

4 participants