-
Notifications
You must be signed in to change notification settings - Fork 25
feat: only_publish_on_change #293
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
base: development
Are you sure you want to change the base?
Conversation
return False | ||
|
||
else: | ||
logging.info(f"Publishing regardless of RECIPE or ARTIFACT diff: {self.project_config.component_name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should default to publishing regardless of recipe/artifact diff if any specific component version is specified in the gdk-config file. IMO, we only care about checking diffs if the version is set to NEXT_PATCH, as this will keep incrementing the patch version and creating new components. If someone specifies an actual component version such as 1.0.0 or 2.0.0 we should attempt the publish to that version regardless of differences.
I'm thinking in some scenario someone may be developing with NEXT_PATCH (say along a 1.0.0 major/minor version), then decide they want to publish the same unchanged component to 2.0.0 to move up a major version and test with other components looking for a version in that dependency range. Unless they remove the only_on_change part of their config, we wouldn't make the publish as it would still be the same component as the latest patch version in the 1.0.0 major/minor version which is still latest. It seems more useful to me for GDK to attempt this publish regardless, rather than prevent a publish to an explicitly defined version. Of course, since only_on_change is not default, they would have added that to their config file at some point themselves as well, so I'm open to hearing your thoughts on what the behavior should be when version isn't set to NEXT_PATCH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking as bellow for scenarios:
version is NEXT_PATCH and only_on_change IS NOT set
- Then continue to publish as per existing behavior.
version is NEXT_PATCH and only_on_change IS set
- Then check for changes against the latest published version.
- only publish if there are changes
version is NOT NEXT_PATCH and only_on_change IS NOT set
- Then continue to publish as per existing behavior.
version is NOT NEXT_PATCH and only_on_change IS set (this part was not in the initial PR)
- Then check for changes against the published version that is set by version.
- If the version does not exists publish
- If the version does exists only publish if there are changes (more efficient)
--
Happy to leave the last one out and also stick to existing behavior, let me know if you think its something needed or not. and i'll make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first three behaviors looks good to me. For the last scenario (version is NOT NEXT_PATCH and only_on_change IS set), if the version does exist, even if there are changes those changes would never be reflected in the component version, as we get something along the lines of the following error:
botocore.errorfactory.ConflictException: An error occurred (ConflictException) when calling the CreateComponentVersion operation: Component [componentname : componentversion] for account [000000000000] already exists with state: [DEPLOYABLE]
A customer would have to delete this component version before GDK could publish to it again with changes. The GDK currently doesn't do this, and we typically recommend customers to create new component versions when they have changes, so I think this error is sufficient as it describes the issue clearly. So checking if there are changes or not both results in the component version remaining unchanged if it already existed. I think leaving the last scenario out and just sticking to existing behavior would essentially have the same effect since the new publish doesn't actually change the component version even if there are changes.
Maybe an improvement here could be to warn in the GDK output if the S3 artifacts have been changed but the version already existed, since component artifacts that are uploaded could result in a different hash from the existing cloud component version's artifact details in the old recipe if they have been modified. But this would be a separate improvement not closely tied to the new only_on_change feature.
Better to remove DeepDiff as a dependency and implement our own recursive diff on the recipe. |
Updated now and removed DeepDiff from requirements.txt |
Description:
This PR allows devs to run the
gdk component publish
command and restrict publishing new versions of the component only when changes are found in the Recipe or Artifact or both.Feature is controlled by the only_on_change option in the publish section of the gdk config. e.g:
"publish": { "bucket": "ggv2artifacts", "region": "eu-west-2", "options": { "only_on_change": ["RECIPE","ARTIFACTS"] } }
This change necessary:
As if you use
gdk component publish
in a pipeline, you do not want to publish if there have been no changes. Currently it always publish and this leads to having a new version of the component for every pipeline run.This change tested:
make tests_unit
and expanded to cover the new options.make tests_integration
and adapted to cover the new options.make tests_unit
Additional information:
Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.