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.

Sign up for GitHub

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

Jump to bottom

Add openstack excon settings #14172

Merged
merged 1 commit into from
Mar 15, 2017
Merged

Add openstack excon settings #14172

merged 1 commit into from
Mar 15, 2017

Conversation

tzumainn
Copy link
Contributor

@tzumainn tzumainn commented Mar 3, 2017 •
edited
Loading

adds read_timeout and omit_default_port excon settings for use with OpenStack providers

https://bugzilla.redhat.com/show_bug.cgi?id=1413912
https://bugzilla.redhat.com/show_bug.cgi?id=1417273

@tzumainn tzumainn mentioned this pull request Mar 3, 2017
Merged
@aufi
Copy link
Member

aufi commented Mar 6, 2017

Looks good to me 👍

blomquisg
blomquisg suggested changes Mar 6, 2017
View reviewed changes
config/settings.yml Outdated
@@ -930,6 +930,9 @@
- MoveIntoResourcePool
:VmSuspendedEvent:
- SuspendVM_Task
:excon:
:openstack_omit_default_port: true
:openstack_read_timeout: 60
Copy link
Member

Choose a reason for hiding this comment

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

Can this excon section be moved to the :ems section?

So, it would look something like:

:ems
  :ems_openstack
    :excon
      :omit_default_port: true
      :read_timeout: 60

Then, accessing looks like:

extra_options[:omit_default_port] = ::Settings.ems.ems_openstack.excon.omit_default_port

The nesting under excon is up to you. As long as the settings are contained under ems_openstack. Then, when the provider is extracted, these settings are easier to track down.

@tzumainn tzumainn force-pushed the openstack-excon-options branch from 21e47b6 to b148c3b Compare March 7, 2017 15:08
@tzumainn tzumainn force-pushed the openstack-excon-options branch from b148c3b to 15510eb Compare March 7, 2017 15:09
@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2017

Checked commit tzumainn@15510eb with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 👍

@tzumainn
Copy link
Contributor Author

tzumainn commented Mar 7, 2017

Oh! sorry, and thanks for the review; I had to rebase and push changes to the branch in order to do a functional test; I'll update when those tests are successful!

@tzumainn
Copy link
Contributor Author

tzumainn commented Mar 7, 2017

Manual testing is succesful! If you think the changes are fine, I guess it might be ready to merge... ?

@blomquisg blomquisg merged commit 0c6b077 into ManageIQ:master Mar 15, 2017
@blomquisg blomquisg added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 15, 2017
@tzumainn
Copy link
Contributor Author

@miq-bot add_label euwe/yes

simaishi pushed a commit that referenced this pull request Mar 16, 2017
@blomquisg @simaishi
Merge pull request #14172 from tzumainn/openstack-excon-options
23d59bb 
Add openstack excon settings
(cherry picked from commit 0c6b077)

https://bugzilla.redhat.com/show_bug.cgi?id=1432644
https://bugzilla.redhat.com/show_bug.cgi?id=1433094
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 23d59bb18e9f69852dd41b21f6dea38e0b31c534
Author: Greg Blomquist <[email protected]>
Date:   Wed Mar 15 10:27:34 2017 -0400

    Merge pull request #14172 from tzumainn/openstack-excon-options
    
    Add openstack excon settings
    (cherry picked from commit 0c6b077693da0f302744f5fb1afa936a91f30acb)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1432644
    https://bugzilla.redhat.com/show_bug.cgi?id=1433094

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blomquisg blomquisg blomquisg approved these changes

Assignees
No one assigned
Labels
enhancement euwe/backported providers/cloud
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tzumainn @aufi @miq-bot @simaishi @blomquisg @Fryguy @chessbyte