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

Add Azure events for targeted refresh #261

Merged
merged 1 commit into from
Mar 15, 2018

Conversation

tumido
Copy link
Member

@tumido tumido commented Mar 13, 2018

Provide automated refresh on events which are recognized by event_target_parser in Azure.

Related to: ManageIQ/manageiq-providers-azure#222

Part of fix for https://bugzilla.redhat.com/show_bug.cgi?id=1487602

@gmcculloug
Copy link
Member

@lfu Please review

@miq-bot
Copy link
Member

miq-bot commented Mar 13, 2018

Checked commit tumido@3b3915c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
10 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

👍 looks great

@gmcculloug
Copy link
Member

@tumido Any reason to keep the wip label?

@tumido
Copy link
Member Author

tumido commented Mar 14, 2018

Just checking if the events get propagated correctly.

We have some events which are inconsistent in names e.g.: publicIPAddresses_delete_EndRequest and publicIpAddresses_delete_EndRequest. So I'm checking if it has any impact. Maybe you can help me? 😉

Does the name field in the YAML has any importance? I know the proper YAML file is located based on downcased matching against event_type, so we should be fine at this point. However, I'm uncertain if it can or cannot cause any harm in any step after that...

We can't have multiple files for the same event, which differs in case-sensitive matching, however downcased are the same. (I know this is our mess and we have to clean it, but we can't do so at this point... Because: reasons, lol.)

@Ladas
Copy link
Contributor

Ladas commented Mar 15, 2018

This is actually Azure's mess, they don't keep consistent resource type across events (event though it's documented correctly). But I am almost sure we pick the handle by the down-cased string, right @gmcculloug @lfu ?

@tumido
Copy link
Member Author

tumido commented Mar 15, 2018

So, I've experimentally confirmed, the events are picked up correctly and the refresh lands fine in the MiqQueue. Removing wip. Thanks for the patience. We want to be sure what we're doing, before we break things, lol. 🍀

@miq-bot remove_label wip

@tumido tumido changed the title [WIP] Add Azure events for targeted refresh Add Azure events for targeted refresh Mar 15, 2018
@miq-bot miq-bot removed the wip label Mar 15, 2018
@gmcculloug
Copy link
Member

@tumido Thanks for testing that out.

@gmcculloug gmcculloug merged commit af63ed2 into ManageIQ:master Mar 15, 2018
@gmcculloug gmcculloug added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 15, 2018
@tumido
Copy link
Member Author

tumido commented Mar 19, 2018

@miq-bot add_label gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Mar 22, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit eee3cc8ece367ff2b878f8ebb6b00d7fec151608
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Mar 15 14:36:09 2018 -0400

    Merge pull request #261 from tumido/azure_events
    
    Add Azure events for targeted refresh
    (cherry picked from commit af63ed2d5c8942985e4389e4960ae0ac3a22c01b)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1558078

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

6 participants