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

Add a project method to Images #405

Open
Temikus opened this issue Jul 22, 2018 · 6 comments
Open

Add a project method to Images #405

Temikus opened this issue Jul 22, 2018 · 6 comments

Comments

@Temikus
Copy link
Member

Temikus commented Jul 22, 2018

See:

 # This attribute is not available in the representation of an
        # 'image' returned by the GCE server (see GCE API). However,
        # images are a global resource and a user can query for images
        # across projects. Therefore we try to remember which project
        # the image belongs to by tracking it in this attribute.
        attribute :project
def get(identity, project = nil)
          if project
            image = service.get_image(identity, project).to_h
            image[:project] = project
            return new(image)
          elsif identity
            project.nil? ? projects = all_projects : projects = [project]
            projects.each do |proj|
              begin
                response = service.get_image(identity, proj).to_h
                # TODO: Remove this - see #405
                response[:project] = proj
                image = response
                return new(image)
              rescue ::Google::Apis::ClientError => e
                next if e.status_code == 404
                break
              else
                break
              end
            end
            # If nothing is found - return nil
            nil
          end
        end

This is really not optimal. This information is already available in self_link and we should just add an object method instead of mixing it in during get request.

@Temikus Temikus added this to the 2.0 milestone Jul 22, 2018
@icco
Copy link
Member

icco commented Jul 22, 2018

Could you expand how this information is already available? The main issue iirc is that we had images across lots of projects that people were referencing by name.

@Temikus
Copy link
Member Author

Temikus commented Jul 23, 2018

@icco If you look closely at the flow it's kinda weird.

get() method logic:

  1. Get the list of all available projects and iterate.
  2. Take next project.
  3. Check if get_image() returns something.
    If it does:
    a) Get the image parameter hash.
    b) Add the :project key to it and populate it with the project we found the image in.
    c) Create new image object from parameters.
    If it doesn't - rescue the 404 and go back to 2.

All the while the image self_link already contains the image reference with the project in it, e.g.:

https://www.googleapis.com/compute/v1/projects/centos-cloud/global/images/centos-7-v20180716

, already contains the centos-cloud project.

In this case IMO it would be more logical to get the parameter out of something the API always provides rather than rely on brittle logic in get() to always set it.

I may be missing something though. So if I am - just let me know :)

Repro:

λ bundle exec rake console
[1] pry(main)> connection = Fog::Compute.new(:provider => "Google")
[2] pry(main)> connection.images.get('centos-7-v20180716')
=>   <Fog::Compute::Google::Image
    name="centos-7-v20180716",
    archive_size_bytes=13890626560,
    creation_timestamp="2018-07-17T10:04:24.974-07:00",
    deprecated=nil,
    description="CentOS, CentOS, 7, x86_64 built on 20180716",
    disk_size_gb=10,
    family="centos-7",
    guest_os_features=nil,
    id=4998083998998830967,
    image_encryption_key=nil,
    kind="compute#image",
    licenses=["https://www.googleapis.com/compute/v1/projects/centos-cloud/global/licenses/centos-7"],
    raw_disk={:container_type=>"TAR", :source=>""},
    self_link="https://www.googleapis.com/compute/v1/projects/centos-cloud/global/images/centos-7-v20180716",
    source_disk=nil,
    source_disk_encryption_key=nil,
    source_disk_id=nil,
    source_image=nil,
    source_image_encryption_key=nil,
    source_image_id=nil,
    source_type="RAW",
    status="READY",
    project="centos-cloud"
  >

@icco
Copy link
Member

icco commented Jul 23, 2018

So are you proposing just removing step 3?

@Temikus
Copy link
Member Author

Temikus commented Jul 24, 2018

@icco I propose:

  1. Removing 3.b
  2. Adding a project method that will parse the self-link and return the image project, making the change effectively a no-op for users.

@Temikus
Copy link
Member Author

Temikus commented Jul 24, 2018

Sorry for dragging this out a bit I think I need to get better at writing bugs 😭

@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants