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

Error in Ruby's rescue clause #20

Closed
mouchtaris opened this issue Apr 21, 2015 · 8 comments · Fixed by #21
Closed

Error in Ruby's rescue clause #20

mouchtaris opened this issue Apr 21, 2015 · 8 comments · Fixed by #21
Assignees

Comments

@mouchtaris
Copy link

In the following file/commit/line:

1976f5c#commitcomment-10822546

Causes (at least) the following error:

/.../ruby/2.2.0/gems/fog-google-0.0.3/lib/fog/google/models/compute/images.rb:67:in `rescue in get': class or module required for rescue clause (TypeError)
    from /.../ruby/2.2.0/gems/fog-google-0.0.3/lib/fog/google/models/compute/images.rb:47:in `get'
    from /.../ruby/2.2.0/gems/fog-google-0.0.3/lib/fog/google/requests/compute/insert_disk.rb:77:in `insert_disk'
    from /.../ruby/2.2.0/gems/fog-google-0.0.3/lib/fog/google/models/compute/disk.rb:40:in `save'
    from /.../ruby/2.2.0/gems/fog-core-1.30.0/lib/fog/core/collection.rb:51:in `create'
    from ...
@plribeiro3000
Copy link
Member

@mouchtaris Thank you for your feedback!

@ihmccreery Perhaps you want to check this out?

@ikehz ikehz self-assigned this Apr 21, 2015
@ikehz
Copy link
Member

ikehz commented Apr 21, 2015

Hi @mouchtaris. Sorry you're having trouble. We're having a bit of a bumpy ride as we move things around in Fog.

Can you give more context about how this came up? What code did you try to execute that gave you this error, so I can try to reproduce it? It looks like you were trying to create a disk. What requires did you use?

@mouchtaris
Copy link
Author

Hello.

Regardless of the conditions that reproduce this error, this code fragment is invalid Ruby code, and in fact it is not a simple typo. The original purpose of the author is completely unclear, and I think it should be made clear what this code is supposed to do in case of some error before testing how it actually behaves with it.

Nevertheless, for me the error is easily reproducible if I pass a Fixnum value for the source_image parameter.

google_client = Google::APIClient.new(
  application_name: 'app_name',
  application_version: 'app_version',
  ).tap { |client|
    client.authorization = :google_app_default
    client.authorization.fetch_access_token!
  }

Fog::Compute::Google.new(
  google_client: google_client,
  google_project: 'project'
  ).disks.create zone: 'europe-west1-c', name: 'disk', size_gb: 10, source_image: 1200

=> TypeError: class or module required for rescue clause
from /.../ruby/2.2.0/gems/fog-google-0.0.3/lib/fog/google/models/compute/images.rb:67:in `rescue in get'

@mouchtaris
Copy link
Author

I am available for any other enquiry.

@ikehz
Copy link
Member

ikehz commented Apr 21, 2015

Thanks for the spot & explanation @mouchtaris; I didn't originally see that it's trying to create a new object.

It looks like this originally made its way into the codebase in fog/fog@39188c3.

I'm continuing my investigation for a fix, but I thought I'd pass along that information at least, since @mouchtaris was wondering what the intent was.

@ikehz
Copy link
Member

ikehz commented Apr 21, 2015

Okay, I've reproduced the bug.

Reverting that block of code to the parent commit fixes it, but, somewhat unrelatedly, the images functionality is currently broken regardless, as the API has changed, but the codebase hasn't reflected that change. I actually started working on the fix yesterday, but haven't finished yet.

With the reversion, @mouchtaris's code will now give the following:

ArgumentError: Parameter 'image' has an invalid value: 1200. Must match: /^[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?$/.

I will continue work on fixing images to work with the API.

ikehz pushed a commit to ikehz/fog-google that referenced this issue Apr 21, 2015
… make it consistent with desired functionality; closes fog#20
@ikehz
Copy link
Member

ikehz commented Apr 21, 2015

The desired functionality is: if we can't find the disk, it should return nil, not raise an error, so I've gone ahead and changed it. The note above about it being broken due to API changes still applies, unfortunately. I will try to get that fixed by the end of the day today.

ikehz pushed a commit to ikehz/fog-google that referenced this issue Apr 21, 2015
… make it consistent with desired functionality; closes fog#20
@ikehz ikehz closed this as completed in #21 Apr 21, 2015
@ikehz
Copy link
Member

ikehz commented Apr 21, 2015

Correction: the current implementation is not broken. See #22 for updates.

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

Successfully merging a pull request may close this issue.

3 participants