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

Add owner to orch stack model provisioning #18209

Merged
merged 1 commit into from
Nov 16, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Nov 15, 2018

Add owner to orch stacks so that when we retire them, we can have some concept of who the retiree is. This is exact the same's we do for services.

@miq-bot
Copy link
Member

miq-bot commented Nov 15, 2018

Checked commit d-m-u@5621fd7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 16, 2018

@miq-bot add_label enhancement
@miq-bot add_reviewer @bdunne
@miq-bot add_reviewer @mkanoor
@miq-bot add_label hammer/yes
@miq-bot assign @gmcculloug

@miq-bot

This comment has been minimized.

@d-m-u
Copy link
Contributor Author

d-m-u commented Nov 16, 2018

@miq-bot add_reviewer @tinaafitz

@miq-bot miq-bot requested a review from tinaafitz November 16, 2018 14:37
Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

The specs look great.

I'm hoping that you can get rid of the before_validation and it will just work.

Ping me if this breaks / or if this works (so I can merge)

@@ -93,6 +97,10 @@ def stdout(format = nil)
format.nil? ? try(:raw_stdout) : try(:raw_stdout, format)
end

def set_tenant_from_group
Copy link
Member

Choose a reason for hiding this comment

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

Does the OwnershipMixin already do this for you?
(will this code work if you remove this validation and method?)

The logic behind the scenes for set_tenant_from_group is quite complex. Otherwise we end up running quite a few queries for code that is not altering the group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. It won't work if you remove the validation and method.

Also, yeah, they are beautiful tests, aren't they? Stolen directly from you.

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

LGTM 👍 :shipit:

@mkanoor mkanoor merged commit ee71bab into ManageIQ:master Nov 16, 2018
@mkanoor mkanoor added this to the Sprint 99 Ending Nov 19, 2018 milestone Nov 16, 2018
@d-m-u d-m-u deleted the orch_stack_provisioning_set_user branch November 16, 2018 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants