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 virtual definition for default_security_group to OpenStack #17979

Merged

Conversation

AparnaKarve
Copy link
Contributor

Fixes the refresh issue.

Related PR #17975

/cc @agrare @Ladas

@kbrock
Copy link
Member

kbrock commented Sep 12, 2018

While virtual_has_one is better over virtual_attribute, is it possible to use a delegate instead?

RESPONSE: nope. this is not written to be able to use a delegate

@AparnaKarve
Copy link
Contributor Author

@kbrock So moving this virtual_has_one to OpenStack, seems to work for the API.

class ManageIQ::Providers::Openstack::CloudManager::CloudTenant < ::CloudTenant
  virtual_has_one :default_security_group, :uses => :security_groups
  ...
  ...
end

@AparnaKarve AparnaKarve force-pushed the virtual_has_one_default_sec_group branch from a6f0509 to 6e6f2c8 Compare September 12, 2018 19:10
@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2018

Checked commit AparnaKarve@6e6f2c8 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@AparnaKarve
Copy link
Contributor Author

Moving the virtual definition of default_security_group to the OpenStack repo.
(discussed with @kbrock )

ManageIQ/manageiq-providers-openstack#355

@AparnaKarve AparnaKarve changed the title use virtual_has_one for default_security_group for refresh to work Move virtual definition for default_security_group to OpenStack Sep 12, 2018
@agrare agrare closed this Sep 12, 2018
@agrare agrare reopened this Sep 12, 2018
@kbrock
Copy link
Member

kbrock commented Sep 12, 2018

Please wait until ManageIQ/manageiq-providers-openstack#355 is green before merging this.

(just to make sure we aren't merging this under false assumptions)

@AparnaKarve
Copy link
Contributor Author

@kbrock I wonder if it's the other way around...
Merging this PR may fix the specs in ManageIQ/manageiq-providers-openstack#355, or at least fix the refresh problem mentioned here #17975 (comment)

@AparnaKarve
Copy link
Contributor Author

AparnaKarve commented Sep 12, 2018

Verified locally that the OpenStack specs pass with this change and also with ManageIQ/manageiq-providers-openstack#355

@agrare agrare merged commit ed40ac3 into ManageIQ:master Sep 12, 2018
@agrare agrare added this to the Sprint 95 Ending Sep 24, 2018 milestone Sep 12, 2018
@AparnaKarve AparnaKarve deleted the virtual_has_one_default_sec_group branch September 13, 2018 00:09
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.

5 participants