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

WIP: Canary steps #4219

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

WIP: Canary steps #4219

wants to merge 9 commits into from

Conversation

Samze
Copy link
Contributor

@Samze Samze commented Feb 14, 2025

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:

  • An explanation of the use cases your change solves

  • Links to any other associated PRs

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

up do
alter_table(:deployments) do
# TODO: what size?
add_column :canary_steps, String, size: 4096
Copy link
Member

Choose a reason for hiding this comment

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

thought: Should this be a JSON column for Postgres?

@@ -99,6 +114,27 @@ def continuable?
state == DeploymentModel::PAUSED_STATE
end

def canary_step
raise 'canary_step is only valid for canary deloyments' unless strategy == CANARY_STRATEGY
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note Calling this is a programming error so seemed best to raise an exception.


unless canary_steps.nil?
# ensure that canary steps are in the correct format for serialization
self.canary_steps = canary_steps.map(&:stringify_keys)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

todo See if we can remove this check.

@@ -86,6 +90,17 @@ def before_update
super
set_status_updated_at
end
# TODO: documentation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

todo remove this comment, documentation is done.

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