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

[FINE] kubernetes_connect, openshift_connect: add timeout settings #15090

Merged
merged 1 commit into from Jun 8, 2017

Conversation

cben
Copy link
Contributor

@cben cben commented May 15, 2017

Backport of ManageIQ/manageiq-providers-kubernetes#10
plus ManageIQ/manageiq-providers-openshift#8 (unnecessary on master but required in backports)

https://bugzilla.redhat.com/show_bug.cgi?id=1440950 (master)

  1. I suppose bugzilla flags have to be confirmed & cloned first...
  2. bump kubeclient ~> 2.4.0 manageiq-gems-pending#156 must be fine/backported before this (this requires kubeclient 2.4.0)
  3. Then this could pass tests and could land on fine.
  4. Then I'll repeat for EUWE...

cc @simon3z @simaishi.

ManageIQ/manageiq-providers-kubernetes#10 from cben/kubeclient-timeout

kubernetes_connect: add timeout settings
(cherry picked from merge commit ManageIQ/manageiq-providers-kubernetes@1ee90b5)

openshift_connect: use kubernetes timeout settings
(cherry picked from unmerged
ManageIQ/manageiq-providers-openshift#8 - unnecessary on master but required in backports)

Requires kubeclient >= 2.4.0
@miq-bot
Copy link
Member

miq-bot commented May 15, 2017

Checked commit cben@d972272 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@simaishi simaishi self-assigned this May 15, 2017
Copy link
Contributor

@simon3z simon3z left a comment

Choose a reason for hiding this comment

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

@cben
Copy link
Contributor Author

cben commented May 15, 2017

Because given ManageIQ/manageiq-providers-openshift#7 it's both unnecessary and impossible to merge ManageIQ/manageiq-providers-openshift#8. That duplicated code doesn't exist any more on master.
And you said ManageIQ/manageiq-providers-openshift#7 should not be backported being a refactoring (makes sense).

If you have a less messy suggestion, I'm all ears :)

@simon3z
Copy link
Contributor

simon3z commented May 15, 2017

LGTM 👍
cc @simaishi

@simon3z
Copy link
Contributor

simon3z commented May 15, 2017

Because given ManageIQ/manageiq-providers-openshift#7 it's both unnecessary and impossible to merge ManageIQ/manageiq-providers-openshift#8. That duplicated code doesn't exist any more on master.
And you said ManageIQ/manageiq-providers-openshift#7 should not be backported being a refactoring (makes sense).

If you have a less messy suggestion, I'm all ears :)

@cben up to you in this case you can squash that tiny refactor here.

@simaishi
Copy link
Contributor

@cben For Euwe, updating kubeclient to 2.4.0 (ManageIQ/manageiq-gems-pending#156) will need to happen in gems/pending/Gemfile. Please include that change in your Euwe PR.

@simon3z
Copy link
Contributor

simon3z commented May 24, 2017

@cben please make Travis happy.

@cben
Copy link
Contributor Author

cben commented May 24, 2017

See PR description, ManageIQ/manageiq-gems-pending#156 must be backported first.
(on fine manageiq-gems-pending was already split so I expect a clean cherry-pick, @simaishi let me know if that too need a [FINE] pr)

@simaishi
Copy link
Contributor

@cben that looks like it's a clean cherry-pick. I can't backport non-blocker PRs to fine right now. I'll restart Travis after I backport ManageIQ/manageiq-gems-pending#156 in a few weeks.

@simaishi simaishi merged commit b434d12 into ManageIQ:fine Jun 8, 2017
@simaishi simaishi added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 8, 2017
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.

None yet

4 participants