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

Google graph refresh #67

Merged
merged 9 commits into from
Sep 6, 2018
Merged

Google graph refresh #67

merged 9 commits into from
Sep 6, 2018

Conversation

slemrmartin
Copy link
Contributor

@slemrmartin slemrmartin commented Aug 22, 2018

Switching Refresher as ManagerRefresh subclass, added collector/parser/persister classes.
Adding Builder and working with InventoryCollection + InventoryObject

Switching Refresher as ManagerRefresh subclass, added collector/parser/persister classes.
zone_uid = parse_uid_from_url(instance.zone)

# TODO(mslemr) if possible, change to lazy_find (now result needed immediately)
flavor = persister.flavors.find(flavor_uid)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ladas how to remove this find? It makes another call to collector on next line, if there is not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is different call to google api

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, I would expect we'll collect all custom flavors as a list, not 1 by 1. Lets mark this one for later, since it will need more tweaking.

# TODO(mslemr): can't be better solution?
cloud_volume_ems_ref = @cloud_volume_url_to_id[attached_disk[:source]]
persister_cloud_volume = persister.cloud_volumes.find(cloud_volume_ems_ref)
next if persister_cloud_volume.nil?
Copy link
Contributor Author

@slemrmartin slemrmartin Aug 24, 2018

Choose a reason for hiding this comment

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

@Ladas Also don' know how to remove this find. If I replace it with lazy_find, then it will build 'disk' everytime, that is not equal.

Copy link
Contributor

Choose a reason for hiding this comment

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

hm, so this differs to other providers, there we build all disks. Basically if it doesn't have a volume, it's a local disk and we do store those in other providers.

Again, lets mark this for later, it will need an exception in specs (since graph refresh will have a different result)


def load_balancer_pool_members(persister_load_balancer_pool, target_pool)
target_pool.instances.to_a.each do |member_link|
persister_lb_pool_member = persister.load_balancer_pool_members.find(Digest::MD5.base64digest(member_link))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ladas same question as above, with lazy_find, there is no condition when to build lb_pool_member and when not

Copy link
Contributor

Choose a reason for hiding this comment

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

lets do this as the volumes, just create this everytime and change specs

@miq-bot
Copy link
Member

miq-bot commented Sep 3, 2018

Checked commits slemrmartin/manageiq-providers-google@d54cba7~...ed51bfb with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
15 files checked, 2 offenses detected

app/models/manageiq/providers/google/inventory/parser/cloud_manager.rb

app/models/manageiq/providers/google/inventory/parser/network_manager.rb

@slemrmartin slemrmartin changed the title [WIP] Google graph refresh Google graph refresh Sep 3, 2018
@miq-bot miq-bot removed the wip label Sep 3, 2018
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 now 👍

@Ladas Ladas assigned Ladas and unassigned agrare Sep 6, 2018
@Ladas Ladas merged commit d12c3b5 into ManageIQ:master Sep 6, 2018
@Ladas Ladas added this to the Sprint 94 Ending Sept 10, 2018 milestone Sep 6, 2018
@slemrmartin slemrmartin deleted the graph-refresh branch September 7, 2018 07:58
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.

4 participants