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

[MiqFileStorage] Add #magic_number_for #401

Merged

Conversation

NickLaMuro
Copy link
Member

Used to determine the magic number for a remote uri.

Implementation Notes/Details:

  • Only works for magics that are determined by the first N bytes currently.
  • By default, just returns the first 256 bytes
  • The :accepted param can be passed, which allows you to pass a hash with the values of the hash being the magic values (returns matching key)
    • When passed, only the largest magic's size in data will be returned by the request
  • Relies on #download_single being able to support a specific @byte_count to download only what is necessary and writing to a StringIO.

Links

@NickLaMuro
Copy link
Member Author

@miq-bot assign @carbonin

@NickLaMuro
Copy link
Member Author

@miq-bot add_label enhancement, hammer/yes

end

describe "with a hash of accepted magics" do
it "returns key for the passed in" do
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what this spec name means. Is there a typo here somewhere or maybe some missing words?

Copy link
Member Author

Choose a reason for hiding this comment

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

"Nick (LaMuro), you're drunk... go home..."

  • Nick (Carboni)

Yeah... let me fix that...

@NickLaMuro NickLaMuro force-pushed the add-magic-number-for-to-miq-file-storage branch from 909e42c to 19f754c Compare November 7, 2018 21:31

if (magics = options[:accepted])
result = magics.detect do |_, magic|
uri_data.force_encoding(magic.encoding).start_with?(magic)
Copy link
Member Author

Choose a reason for hiding this comment

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

@carbonin Just an FYI, this also changed as part of my recent push of commits, since I ran into an issue without it.

Prior, this line was just:

result = magics.detect { |_, magic| ri_data.start_with?(magic) }

Copy link
Member

Choose a reason for hiding this comment

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

Can we keep this as a single line? Code Climate is complaining about the alignment of the end and even though the line would be over 80 characters I think it would be more readable as a single line rather than dealing with the weird spacing caused by the assignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. No prob.

Used to determine the magic number for a remote uri.  Relies on
`#download_single` being able to support a specific `@byte_count` to
download only what is necessary and writing to a StringIO.
Since we need a connection for any download/upload to work (sets @ftp on
the instance), we need a slight override of `#magic_number_for` to wrap
it with a connection.
@NickLaMuro NickLaMuro force-pushed the add-magic-number-for-to-miq-file-storage branch from 19f754c to f70c48d Compare November 8, 2018 19:37
@miq-bot
Copy link
Member

miq-bot commented Nov 8, 2018

Checked commits NickLaMuro/manageiq-gems-pending@a24d703~...f70c48d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 3 offenses detected

lib/gems/pending/util/miq_file_storage.rb

@carbonin carbonin merged commit b3f99e1 into ManageIQ:master Nov 9, 2018
@carbonin carbonin added this to the Sprint 99 Ending Nov 19, 2018 milestone Nov 9, 2018
simaishi pushed a commit that referenced this pull request Nov 12, 2018
…ile-storage

[MiqFileStorage] Add #magic_number_for

(cherry picked from commit b3f99e1)
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 95c981ff06d3963e95dadd134e4a44d971bddf82
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Fri Nov 9 09:31:27 2018 -0500

    Merge pull request #401 from NickLaMuro/add-magic-number-for-to-miq-file-storage
    
    [MiqFileStorage] Add #magic_number_for
    
    (cherry picked from commit b3f99e12a1daeb9ecdd07173a49be93038da7051)

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

Successfully merging this pull request may close these issues.

4 participants