Fixes and cleanup for MiqSwiftStorage #398
Conversation
There was a bug when passing in a `:uri` to `MiqSwiftStorage` where if
the path for uri contained a filename as well:
swift://example.com/container/path/to/file?query=params
It would assign the `@container_name` to the full path (minus the
leading slash), and not just the top level "directory".
This fixes that so it is correct.
Also adds some spacing for clarity.
|
Checked commits NickLaMuro/manageiq-gems-pending@aa9436d~...10ac180 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 lib/gems/pending/util/object_storage/miq_swift_storage.rb
|
| @@ -16,12 +16,15 @@ def initialize(settings) | |||
| @bucket_name = URI(@settings[:uri]).host | |||
|
|
|||
| raise "username and password are required values!" if @settings[:username].nil? || @settings[:password].nil? | |||
| _scheme, _userinfo, @host, @port, _registry, @mount_path, _opaque, query, _fragment = URI.split(URI.encode(@settings[:uri])) | |||
There was a problem hiding this comment.
Was the @mount_path variable unused before? Seems odd that it was being assigned here if we didn't need it. @jerryk55 ?
There was a problem hiding this comment.
To clarify, I see that it was being used to set @container_name, but because it's an instance variable (rather than a local) I'm being cautious about it being used elsewhere.
There was a problem hiding this comment.
Just took a closer look. Looks like this is only used in mount sessions (rather than object stores).
There was a problem hiding this comment.
Yeah, we should probably remove the @mount_path and just make it a local variable so it can be garbage collected, but I decided to leave it for now. Can do a follow up later to fix.
There was a problem hiding this comment.
Oh, I just looked at the code I wrote, and I did do that... ignore me...
…ft-storage Fixes and cleanup for MiqSwiftStorage (cherry picked from commit 54b6861)
|
Hammer backport details: |
@container_namegeneration (didn't properly handle nested keys