[DARGA] Update armrest gem, update subscription handling and add region handling for Darga #13311
Conversation
|
@miq-bot add_label wip |
|
@chessbyte @bdunne I have no idea what's going on with those failures. Is there something special I'm supposed to do when submitting a PR against a specific branch like this? |
|
Kicking Travis |
|
@bdunne What's the word? |
|
@djberg96 Looks like you have some test failures to address. |
|
This pull request is not mergeable. Please rebase and repush. |
chessbyte
changed the title
| @@ -3,7 +3,7 @@ raise "Ruby versions less than 2.2.2 are unsupported!" if RUBY_VERSION < "2.2.2" | |||
| # VMDB specific gems | |||
| # | |||
|
|
|||
| gem "rails", "~>5.0.0.1" | |||
| gem "rails", "~>5.0.0", ">= 5.0.0.1" | |||
There was a problem hiding this comment.
I don't think you want this change.
the previous version was more restrictive. and the current build failure is due to lightening up that restriction
There was a problem hiding this comment.
Yeah this change allows 5.0.1 which is known not to work. We had agreed that Darga and Euwe should be restricted to 5.0.0 due to incompatibilities with virtual_attributes that we don't want to bother backporting.
miq-bot
changed the title
chrisarcand
changed the title
miq-bot
changed the title
bdunne
changed the title
|
Just a quick update, iit works, but I should note that usgov regions do not support events and metrics. This is a limitation from Azure, not of this library. |
|
@miq-bot remove_label wip |
djberg96
changed the title
|
@djberg96 can you please add a reference to equivalent 'master' PR(s)? |
|
@simaishi #13670 and ManageIQ/manageiq-providers-azure#28. This is a pain because of the repo splitting that's happened between darga, euwe (the azure provider) and master (gems-pending). |
|
Thanks @djberg96 Will wait for an approval from @kbrock or @chrisarcand |
…ion handling. Bump armrest version to 0.5.2 and remove old blank subscription logic. Scope Azure::Armrest::ApiException properly. Update call to clear_caches.
|
Some comments on commit https://github.com/djberg96/manageiq/commit/c37d86c1ee70c83844e56170585ae5a6881d57c7 gems/pending/spec/recordings/miq_vm/miq_azure_vm_image_spec/miq_azure_vm_image_spec_rootTrees-2.yml
|
|
Checked commit https://github.com/djberg96/manageiq/commit/c37d86c1ee70c83844e56170585ae5a6881d57c7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
I'm too unfamiliar to approve and merge this, sorry. I reviewed only the previous change that made Rails update to an unusable version in Darga. |
|
@blomquisg @bronaghs Look good? Can we merge? |
| Class.new do | ||
| def join; end | ||
| end.new | ||
| end | ||
| end |
There was a problem hiding this comment.
@djberg96 why is this no longer needed? What was it for?
There was a problem hiding this comment.
It was for thread safety for VCR. But VCR added their own safety and keeping it caused problems. We set our own thread limit directly within the armrest gem.
|
@bzwei - Might any of the PRs you authored between versions 0.2.11 and 0.5.2 of the azure-armrest gem be incompatible with the current darga branch? This one maybe: |
|
@bronaghs I actually need 0.5.x for the PR |
|
The code changes look good in this PR however I cannot verify there will be no compatibility issues but I expect QE will help with that. @blomquisg - over to you. |
|
@blomquisg Is it safe to merge now? |
miq-bot
changed the title
simaishi
changed the title
This updates the Darga branch to 0.5.x of the azure-armrest gem, putting it in line with the other branches. Not only does this make maintenance between branches easier going forward, it also enables USGov support, and potentially other regions.
Because subscription ID's are technically optional in Darga, some logic changes were required to accommodate changes within the armrest gem itself, as well as the addition of a region argument to the
raw_connectmethod.Addresses https://bugzilla.redhat.com/show_bug.cgi?id=1403366
Marked as a WIP for now, since I need to ensure metrics, events and provisioning work as expected.