fog-google icon indicating copy to clipboard operation
fog-google copied to clipboard

Models should not have more than 1 required parameter in get()

Open Temikus opened this issue 6 years ago • 5 comments

This will need to be fixed, otherwise lifecycle doesn't match:

λ grep -R "def get([a-z_\*-]*, " . 
./lib/fog/google/models/sql/ssl_certs.rb:        def get(instance_id, sha1_fingerprint)
./lib/fog/google/models/sql/backup_runs.rb:        def get(instance_id, backup_run_id)
./lib/fog/storage/google_xml/models/directories.rb:        def get(key, options = {})
./lib/fog/storage/google_xml/models/files.rb:        def get(key, options = {}, &block)
./lib/fog/storage/google_json/models/directories.rb:        def get(bucket_name, options = {})
./lib/fog/storage/google_json/models/files.rb:        def get(key, options = {}, &block)
./lib/fog/compute/google/models/target_pools.rb:        def get(identity, region = nil)
./lib/fog/compute/google/models/addresses.rb:        def get(identity, region)
./lib/fog/compute/google/models/instance_groups.rb:        def get(identity, zone = nil)
./lib/fog/compute/google/models/images.rb:        def get(identity, project = nil)
./lib/fog/compute/google/models/instance_group_managers.rb:        def get(identity, zone = nil)
./lib/fog/compute/google/models/forwarding_rules.rb:        def get(identity, region = nil)
./lib/fog/compute/google/models/operations.rb:        def get(identity, zone = nil, region = nil)
./lib/fog/compute/google/models/disks.rb:        def get(identity, zone)
./lib/fog/compute/google/models/servers.rb:        def get(identity, zone = nil)
./lib/fog/compute/google/models/machine_types.rb:        def get(identity, zone)
./lib/fog/compute/google/models/disk_types.rb:        def get(identity, zone = nil)
./lib/fog/compute/google/models/target_instances.rb:        def get(target_instance, zone = nil)
./lib/fog/compute/google/models/subnetworks.rb:        def get(identity, region)
./lib/fog/dns/google/models/records.rb:        def get(name, type)

Game plan:

  • [x] Unit tests for basic model lifecycle methods.
  • [x] Add lookup logic if the resource location is not provided (probably via all?)
  • [ ] Switch all zone/region parameters to be optional, but leave them in to speed up logic when needed.
  • [ ] Fixup the regression in collection tests (see #396)
  • [x] Unit tests checking for get() required parameters so this doesn't deviate again.
  • [ ] Remove skip test fixtures in test:unit

Temikus avatar Jun 05 '18 22:06 Temikus

I think this may be a good time to think about some unit tests testing the most basic behavior of a model.

I.e. should respond to: .save .destroy

We can get all models loaded via:

ObjectSpace.each_object(Fog::Model.singleton_class) {|it| puts it}
Fog::Model
Fog::Compute::Google::GlobalAddress
Fog::Compute::Google::Zone
Fog::Compute::Google::Region
Fog::Compute::Google::HttpHealthCheck
Fog::Compute::Google::TargetPool
Fog::Compute::Google::ForwardingRule
Fog::Compute::Google::Project
Fog::Compute::Google::Firewall
Fog::Compute::Google::Network
Fog::Compute::Google::Route
Fog::Compute::Google::BackendService
Fog::Compute::Google::TargetHttpProxy
Fog::Compute::Google::TargetHttpsProxy
Fog::Compute::Google::UrlMap
Fog::Compute::Google::GlobalForwardingRule
Fog::Compute::Google::TargetInstance
Fog::Compute::Google::Image
Fog::Compute::Google::Disk
Fog::Compute::Google::MachineType
Fog::Compute::Google::Snapshot
Fog::Compute::Server
Fog::Compute::Google::Server
Fog::Compute::Google::DiskType
Fog::Compute::Google::Address
Fog::Compute::Google::Operation
Fog::Compute::Google::InstanceGroup
Fog::Compute::Google::Subnetwork
Fog::Compute::Google::InstanceTemplate
Fog::Compute::Google::InstanceGroupManager
Fog::Compute::Google::SslCertificate

Same with collection:

ObjectSpace.each_object(Fog::Collection.singleton_class) {|it| puts it}
Fog::Collection
Fog::Association
Fog::PagedCollection
Fog::Compute::Google::Servers
Fog::Compute::Google::Images
Fog::Compute::Google::Disks
Fog::Compute::Google::DiskTypes
Fog::Compute::Google::MachineTypes
Fog::Compute::Google::Addresses
Fog::Compute::Google::GlobalAddresses
Fog::Compute::Google::Operations
Fog::Compute::Google::Snapshots
Fog::Compute::Google::Regions
Fog::Compute::Google::Zones
Fog::Compute::Google::HttpHealthChecks
Fog::Compute::Google::TargetPools
Fog::Compute::Google::Projects
Fog::Compute::Google::ForwardingRules
Fog::Compute::Google::Firewalls
Fog::Compute::Google::Networks
Fog::Compute::Google::BackendServices
Fog::Compute::Google::Routes
Fog::Compute::Google::TargetHttpProxies
Fog::Compute::Google::TargetHttpsProxies
Fog::Compute::Google::GlobalForwardingRules
Fog::Compute::Google::UrlMaps
Fog::Compute::Google::TargetInstances
Fog::Compute::Google::InstanceGroups
Fog::Compute::Google::Subnetworks
Fog::Compute::Google::InstanceTemplates
Fog::Compute::Google::InstanceGroupManagers
Fog::Compute::Google::SslCertificates

Temikus avatar Jun 05 '18 22:06 Temikus

Note to self:

common_ancestors = [Fog::Collection, Fog::Association, Fog::PagedCollection]
descendants = ObjectSpace.each_object(Fog::Collection.singleton_class).to_a

collections = descendants - common_ancestors
common_ancestors = [Fog::Model]
descendants = ObjectSpace.each_object(Fog::Model.singleton_class).to_a

models = descendants - common_ancestors

Temikus avatar Jun 06 '18 00:06 Temikus

This sounds great!

icco avatar Jul 15 '18 14:07 icco

Made some good progress on this in #400. SQL and DNS are the only collections left that have >1 required argument get methods.

Temikus avatar Sep 26 '18 00:09 Temikus

This issue has been marked inactive and will be closed if no further activity occurs.

github-actions[bot] avatar Apr 18 '21 02:04 github-actions[bot]