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

feat: HEXA-1146 Propagate pipeline default parameters in new versions #883

Conversation

YolanFery
Copy link
Contributor

@YolanFery YolanFery commented Dec 20, 2024

Uploading a new pipeline version was overriding config values. This PR is about keeping the previous config values while taking into account parameters that can change.

Changes

  • Add a helper method to get the previous config while considering changes in parameters
  • Unit test covering 3 scenarios

How/what to test

  • Adding new versions keeps the config
  • Adding new versions with different or partially different parameter keeps the config for the previously existing ones

previous_config_from_common_parameter = {}
if self.last_version:
previous_parameters = self.last_version.parameters
common_parameters = [
Copy link
Contributor

@nazarfil nazarfil Dec 23, 2024

Choose a reason for hiding this comment

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

I have only one comment on the naming, that "common" might not reflect the best that we are talking about overlapping/shared parameters between new and previous versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes a lot of sense, thanks, fixed in next commits

@YolanFery YolanFery requested a review from nazarfil December 23, 2024 11:08
@nazarfil nazarfil merged commit 6e518f6 into main Dec 24, 2024
4 checks passed
@nazarfil nazarfil deleted the HEXA-1146-propagate-pipeline-default-parameters-with-new-versions branch December 24, 2024 08:01
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.

2 participants