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

Tower CUD check and run refresh_in_provider followed by refreshing manager #15025

Merged
merged 1 commit into from
May 8, 2017

Conversation

jameswnl
Copy link
Contributor

@jameswnl jameswnl commented May 8, 2017

The targeted refresh is only implemented on confirguration_script_source and so if it is in the context of updating a credential, the refresh on the credential will be dropped.

In prior PR, was convinced by this section that it will fall back to ems refresh automatically.

Now realized, it will be dropped by this queue attempt way before.

@miq-bot add_labels fine/yess, providers/ansible_tower, blocker, bug

@miq-bot
Copy link
Member

miq-bot commented May 8, 2017

@jameswnl Cannot apply the following label because they are not recognized: fine/yess

@jameswnl
Copy link
Contributor Author

jameswnl commented May 8, 2017

@miq-bot add_labels fine/yes

@miq-bot
Copy link
Member

miq-bot commented May 8, 2017

Checked commit jameswnl@ee90bbf with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@blomquisg
Copy link
Member

The targeted refresh is only implemented on confirguration_script_source and so if it is in the context of updating a credential, the refresh on the credential will be dropped.

@Ladas, can you take a look at the graph refresh flow? Need to make sure that if a targeted refresh is requested, but not implemented, then the full refresh is kicked off instead. Can be done as a separate follow-up PR. But, I don't think the callers should care if the targeted refresh exists for specific objects yet. The callers should be able to assume it can call a refresh with a target and the target gets refreshed (even if it means it's a full refresh).

@blomquisg blomquisg merged commit 2b14633 into ManageIQ:master May 8, 2017
@blomquisg blomquisg added this to the Sprint 60 Ending May 8, 2017 milestone May 8, 2017
@Ladas
Copy link
Contributor

Ladas commented May 9, 2017

@blomquisg right, we need to redesign the target refresh queuing, this queues the AR object, so it will be filtered out by the core code. @durandom ^

simaishi pushed a commit that referenced this pull request May 9, 2017
Tower CUD check and run refresh_in_provider followed by refreshing manager
(cherry picked from commit 2b14633)

https://bugzilla.redhat.com/show_bug.cgi?id=1448098
@simaishi
Copy link
Contributor

simaishi commented May 9, 2017

Fine backport details:

$ git log -1
commit 647e49c22715dd11a53011de281f27e0be5f0ba0
Author: Greg Blomquist <[email protected]>
Date:   Mon May 8 17:26:49 2017 -0400

    Merge pull request #15025 from jameswnl/avoid-target-refresh
    
    Tower CUD check and run refresh_in_provider followed by refreshing manager
    (cherry picked from commit 2b14633149392a51c11490555f5f2ecb27bd5794)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1448098

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