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

Update driven refresh #186

Merged
merged 7 commits into from
Apr 5, 2018
Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Feb 20, 2018

This change takes the existing cache-less update driven refresh and cached all of the inventory in order to fix a number of issues with parsing to our schema without having all of the inventory cached.

The goal here is to get the update-driven part of this style of refresh in then work on reducing the cache footprint instead of trying to get it all in at once.

As much as was possible I took the cache code from the existing VimBroker but I added tests which it never had https://github.com/ManageIQ/manageiq-providers-vmware/pull/186/files#diff-e11e8b8028111e573bec9a54002835ff

@miq-bot miq-bot added the wip label Feb 20, 2018
@agrare agrare force-pushed the update_driven_refresh branch from 2e57b29 to ece190c Compare March 5, 2018 14:18
@agrare agrare force-pushed the update_driven_refresh branch 3 times, most recently from eb2aef6 to 45506c7 Compare March 21, 2018 17:46
@ManageIQ ManageIQ deleted a comment from miq-bot Mar 21, 2018
@agrare agrare force-pushed the update_driven_refresh branch 4 times, most recently from e22e65a to d98fca6 Compare March 23, 2018 17:36
@agrare agrare force-pushed the update_driven_refresh branch from d98fca6 to b3049f0 Compare April 3, 2018 20:04
@agrare agrare changed the title [WIP] Update driven refresh Update driven refresh Apr 4, 2018
@miq-bot miq-bot removed the wip label Apr 4, 2018
@agrare agrare force-pushed the update_driven_refresh branch 2 times, most recently from fdc6b8a to b43dbf7 Compare April 4, 2018 18:55
@miq-bot
Copy link
Member

miq-bot commented Apr 4, 2018

Checked commits agrare/manageiq-providers-vmware@d8f0d40~...b43dbf7 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
13 files checked, 0 offenses detected
Everything looks fine. 🏆

@agrare agrare force-pushed the update_driven_refresh branch from b43dbf7 to ca1252f Compare April 4, 2018 19:34
enabled = props["configuration.drsConfig.enabled"]
cluster_hash[:drs_enabled] = enabled.to_s.downcase == "true"
end
if props.include?("configuration.drsConfig.defaultVmBehavior")
Copy link
Contributor

Choose a reason for hiding this comment

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

so, we will set a nil, instead of not having the :key ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that we're caching the properties we don't have to worry about getting a sparse update.
When we move to not cache some properties we'll need to go back to checking for every key that we don't cache but this way I find is more readable in the meant time

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.

👍 awesome change, we'll go with incremental improvements, so we do not need to cache the full state

@Ladas Ladas merged commit 3b18575 into ManageIQ:master Apr 5, 2018
@agrare agrare deleted the update_driven_refresh branch April 10, 2018 12:25
@Ladas Ladas added this to the Sprint 84 Ending Apr 23, 2018 milestone Apr 24, 2018
agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
Revert "Support TTL (Time To Live) value for services."
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.

3 participants