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

Add persistor with configurable saver strategy #73

Merged
merged 11 commits into from
Jul 27, 2017
Merged

Add persistor with configurable saver strategy #73

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
07971aa
Add inventory persister for kubernetes
agrare Jun 19, 2017
dd12ac7
Expose shared options of the existing ICs
Ladas Jul 14, 2017
9c21f48
Use a Persistor in the Parser
Ladas Jul 25, 2017
3d20bac
Make InventoryCollections configurable by Settings
Ladas Jul 25, 2017
a378d68
Test :default and :batch saver strategies
Ladas Jul 25, 2017
5bfd4f7
Blacklist tags for container routes and builds
Ladas Jul 26, 2017
3a2e8bd
Deduplicate spec before blocks
Ladas Jul 26, 2017
b2b618d
Remove image labels as those are conditionally in the OSE parser
Ladas Jul 26, 2017
f22f947
Extract persister class, so it can be redefined by OpenShift
Ladas Jul 26, 2017
8da447f
ContainerService id is part of the ContainerServicePortConfig ems_ref
Ladas Jul 27, 2017
0af5990
Add todo for :protocol inside of ContainerServicePortConfig manager_ref
Ladas Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ module ManageIQ::Providers::Kubernetes
class ContainerManager::RefreshParser
include Vmdb::Logging
include ContainerManager::EntitiesMapping
include ContainerManager::InventoryCollections

def self.ems_inv_to_hashes(inventory, options = Config::Options.new)
new(options).ems_inv_to_hashes(inventory, options)
Expand Down Expand Up @@ -41,8 +40,12 @@ def ems_inv_to_hashes(inventory, _options = Config::Options.new)
@data
end

def persister_class
ManageIQ::Providers::Kubernetes::Inventory::Persister::ContainerManager
Copy link
Contributor

Choose a reason for hiding this comment

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

A job for Inventory.persister_class_for() ?
(I'm actually happy as written — less magic to follow, but if you already have the magic lookup pattern in other places...)

Copy link
Contributor Author

@Ladas Ladas Jul 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

possibly, but the next step should be to expose persistor, instead of @inv_collections. So yeah, we could abstract that on the Inventory.

end

def ems_inv_to_inv_collections(ems, inventory, options = Config::Options.new)
persister = ManageIQ::Providers::Kubernetes::Inventory::Persister::ContainerManager.new(ems)
persister = persister_class.new(ems)
# TODO expose Persistor and use that
Copy link

Choose a reason for hiding this comment

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

@Ladas can you please explain this TODO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just that we should replace the usage of @inv_collections, that we have in many files by the Persister object, that should be the interface

Copy link

Choose a reason for hiding this comment

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

ahh okay, was a little confused by the fact that here the todo is already done...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, we just need to fix all the other places. :-)

@inv_collections = persister.collections
Copy link
Contributor

Choose a reason for hiding this comment

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

https://english.stackexchange.com/questions/206893/persister-or-persistor ?
(unconclusive, some anecdotal usage but no etymological basis for distinct meanings)


Expand Down