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

Cloud database servers refresh support #513

Merged
merged 1 commit into from
Jul 14, 2022
Merged

Cloud database servers refresh support #513

merged 1 commit into from
Jul 14, 2022

Conversation

nasark
Copy link
Member

@nasark nasark commented Jul 7, 2022 •
edited by agrare
Loading

i.e.

 #<ManageIQ::Providers::Azure::CloudManager::CloudDatabaseServer:0x00007febc4205f78
  id: 9,
  name: "miq-sql-server",
  ems_ref:
   "/subscriptions/2b01dec2-6684-4f25-abfc-a2453e0ae52f/resourceGroups/test-group/providers/Microsoft.Sql/servers/miq-sql-server",
  ems_id: 15,
  server_type: "SQL",
  region: "East US",
  status: "Ready",
  version: "12.0",
  resource_group_id: 6>,

Dependent:

@miq-bot assign @agrare
@miq-bot add_label enhancement

@nasark nasark mentioned this pull request Jul 7, 2022
Merged
2 tasks
@nasark nasark mentioned this pull request Jul 7, 2022
Merged
2 tasks
@nasark nasark changed the title Cloud database servers refresh support [WIP] Cloud database servers refresh support Jul 7, 2022
@miq-bot miq-bot added the wip label Jul 7, 2022
@agrare
Copy link
Member

agrare commented Jul 11, 2022

@miq-bot cross-repo-tests ManageIQ/manageiq#21977 ManageIQ/manageiq-schema#653

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 11, 2022
@agrare @miq-bot
Running tests for agrare
24e64e9 
From Pull Request:  ManageIQ/manageiq-providers-azure#513
@miq-bot miq-bot mentioned this pull request Jul 11, 2022
Closed
@nasark nasark changed the title [WIP] Cloud database servers refresh support Cloud database servers refresh support Jul 11, 2022
@miq-bot miq-bot removed the wip label Jul 11, 2022
Fryguy
Fryguy reviewed Jul 11, 2022
View reviewed changes
app/models/manageiq/providers/azure/inventory/parser/cloud_manager.rb Outdated
:name => server.name,
:server_type => 'SQL',
:region => server.location,
:status => server.properties&.state,
Copy link
Member

Choose a reason for hiding this comment

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

@agrare Do we need some sort of normalization on the status field? Or perhaps a raw_status vs normalized_status?

Copy link
Member

Choose a reason for hiding this comment

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

👍 yeah we should map all of the possible azure states onto a a set that would work across providers

Copy link
Member Author

@nasark nasark Jul 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@agrare @Fryguy I was not able to replicate any other states aside from "Ready". The Azure docs also do not provide the possible states https://docs.microsoft.com/en-us/rest/api/sql/2021-11-01-preview/servers/get#server. As a result, I have left it as https://github.com/ManageIQ/manageiq-providers-azure/pull/513/files#diff-0d177831b5af9863211ff4fa5e8c6810dac1b79b9cf5e132680b8db96f7b6f5cR573-R580. Since deleted servers don't persist on Azure and "Ready" seems to be the only available state, I am wondering whether the status column is even necessary at this point? Let me know

Fryguy
Fryguy reviewed Jul 11, 2022
View reviewed changes
app/models/manageiq/providers/azure/inventory/parser/cloud_manager.rb Outdated
:ems_ref => server.id,
:name => server.name,
:server_type => 'SQL',
:region => server.location,
Copy link
Member

Choose a reason for hiding this comment

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

I think the region was removed in the schema.

@nasark nasark force-pushed the cloud_database_servers_refresh_support branch from 96c21ca to d5a11bb Compare July 12, 2022 14:18
@nasark

This comment was marked as outdated.

Sign in to view
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 12, 2022
@miq-bot
Running tests for nasark
5830af4 
From Pull Request:  ManageIQ/manageiq-providers-azure#513
@miq-bot miq-bot mentioned this pull request Jul 12, 2022
Closed
@nasark
Copy link
Member Author

nasark commented Jul 12, 2022

@miq-bot cross-repo-tests ManageIQ/manageiq#21977 ManageIQ/manageiq-schema#653

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 12, 2022
@miq-bot
Running tests for nasark
48f99f6 
From Pull Request:  ManageIQ/manageiq-providers-azure#513
@miq-bot miq-bot mentioned this pull request Jul 12, 2022
Closed
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request Jul 12, 2022
@miq-bot
Running tests for nasark
1bcc766 
From Pull Request:  ManageIQ/manageiq-providers-azure#513
@miq-bot miq-bot mentioned this pull request Jul 12, 2022
Closed
@nasark nasark force-pushed the cloud_database_servers_refresh_support branch from d5a11bb to ed99303 Compare July 12, 2022 16:00
@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2022

Checked commit nasark@ed99303 with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
4 files checked, 0 offenses detected
Everything looks fine. ⭐

@nasark
Copy link
Member Author

nasark commented Jul 12, 2022 •
edited
Loading

@agrare Aside from this comment #513 (comment), I think this PR and all its dependents are ready for another look. Thanks

@agrare agrare closed this Jul 14, 2022
@agrare agrare reopened this Jul 14, 2022
@agrare agrare merged commit 6a16243 into ManageIQ:master Jul 14, 2022
@MelsHyrule MelsHyrule mentioned this pull request Jul 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fryguy Fryguy Fryguy left review comments

@agrare agrare agrare approved these changes

@djberg96 djberg96 Awaiting requested review from djberg96 djberg96 is a code owner

Assignees

@agrare agrare

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nasark @agrare @miq-bot @Fryguy