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

Scope the default zone to the current region #17103

Merged
merged 1 commit into from
Mar 8, 2018

Conversation

gtanzillo
Copy link
Member

This addresses the case where a new server is added to a DB containing multiple regions and the default
zone from a remote region is assigned

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1552231

@gtanzillo gtanzillo added the bug label Mar 7, 2018
let!(:default_in_my_region) { described_class.create(:name => "default", :description => "Default Zone") }

it ".default_zone returns a zone in the current region" do
expect(described_class.default_zone).to eq(default_in_my_region)
Copy link
Member Author

Choose a reason for hiding this comment

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

Anyone have a suggestion on how to ensure that this test does not result in a false positive since find_by(:name => "default") could end up returning the one we want, in the current region without the scoped query

Copy link
Member

Choose a reason for hiding this comment

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

No, I think we have to depend on sporadic test failures to find that.

Copy link
Member

Choose a reason for hiding this comment

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

It may be possible to create a method/block in_region(ApplicationRecord.my_region_number + 1) do ; end
Seems like overkill. I like Brandon's response

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

👍 LGTM

let!(:default_in_my_region) { described_class.create(:name => "default", :description => "Default Zone") }

it ".default_zone returns a zone in the current region" do
expect(described_class.default_zone).to eq(default_in_my_region)
Copy link
Member

Choose a reason for hiding this comment

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

No, I think we have to depend on sporadic test failures to find that.

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.

:shipit:

let!(:default_in_my_region) { described_class.create(:name => "default", :description => "Default Zone") }

it ".default_zone returns a zone in the current region" do
expect(described_class.default_zone).to eq(default_in_my_region)
Copy link
Member

Choose a reason for hiding this comment

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

It may be possible to create a method/block in_region(ApplicationRecord.my_region_number + 1) do ; end
Seems like overkill. I like Brandon's response

This addresses the case where a new server is added to a DB containing multiple regions and the default
zone from a remote region is assigned

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1552231
@gtanzillo gtanzillo force-pushed the scope-default-zone-to-region branch from f2259ad to be2a49a Compare March 8, 2018 15:01
@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2018

Checked commit gtanzillo@be2a49a 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. ⭐

@kbrock kbrock merged commit 8a4447a into ManageIQ:master Mar 8, 2018
@agrare agrare added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 13, 2018
@simaishi
Copy link
Contributor

@gtanzillo Can this be gaprindashvili/yes?

simaishi pushed a commit that referenced this pull request Apr 12, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 8dcd851dda256276b8562b30ca311092aaf6e78e
Author: Keenan Brock <[email protected]>
Date:   Thu Mar 8 14:35:08 2018 -0500

    Merge pull request #17103 from gtanzillo/scope-default-zone-to-region
    
    Scope the default zone to the current region
    (cherry picked from commit 8a4447a323c0d34b6493f44b891c69ab85c1e3d8)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1566568

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