Improve instance_types.rake logic

[jameswnl]

I have submitted this PR. That PR will enhance the site's json output so that we can eliminate the tedious manual step to curl and using 3rd party site to convert html to csv

Meanwhile we can use my forked copy and run python scrape.py locally to generate the json file.

update

The PR to http://ec2instances.info has been merged

[jameswnl]

@durandom Would like to get your input on this before un-wip it.

@miq-bot add_label wip, enhancement

[durandom]

great idea to add the parsing to ec2instances upstream!

[durandom]

is this a problem, that this is now a string?
is the db a string?
will it impact UI sorting?

[jameswnl]

The original code was

o[:processor_clock_speed] = row[:clock_speed_ghz].scan(/[\d\.]/).join

And that won't work because some of the new values of row[:clock_speed_ghz] are like '2.3 to 2.7'. The scan will return '2.32.7' which doesn't make sense and also breaks the numeric field.

This is the part need your help. Do you know how we use this? Or who would know that I can ask?

[jameswnl]

Actually this won't matter, the db doesn't care about this, it only cares about

new_result = {
      :type                     => ManageIQ::Providers::Amazon::CloudManager::Flavor.name,
      :ems_ref                  => uid,
      :name                     => name,
      :description              => flavor[:description],
      :enabled                  => !flavor[:disabled],
      :cpus                     => cpus,
      :cpu_cores                => 1,
      :memory                   => flavor[:memory],
      :supports_32_bit          => flavor[:architecture].include?(:i386),
      :supports_64_bit          => flavor[:architecture].include?(:x86_64),
      :supports_hvm             => flavor[:virtualization_type].include?(:hvm),
      :supports_paravirtual     => flavor[:virtualization_type].include?(:paravirtual),
      :block_storage_based_only => flavor[:ebs_only],
      :cloud_subnet_required    => flavor[:vpc_only],
      :ephemeral_disk_size      => flavor[:instance_store_size],
      :ephemeral_disk_count     => flavor[:instance_store_volumes]
    }

as in parsing code here

[durandom]

is :network_performance a free text? IIRC it should be rather a set of [:high, :low, etc]

[jameswnl]

Similarly, there are new values breaking the original parsing logic which was

 -    o[:network_performance] = case i[:network_performance]
 -                              when '10 Gigabit', '20 Gigabit'
 -                                :very_high
 -                              else
 -                                i[:network_performance].downcase.tr(' ', '_').to_sym
 -                              end

Yes, AWS use free text for this field. So it's hard to parse it into structured type.

[bronaghs]

I dont think we actually use this.

[durandom]

I'd leave this comment in the code as a pointer to what 'ebs_only' means

[jameswnl]

sure, done!

[durandom]

I see that this is now included in ec2instances.info, which is great

[jameswnl]

Yes, they just merged my vantage-sh/ec2instances.info#270 hours ago 😉

[durandom]

but this isnt included...
did you remove it, because we assume that previous generations do not change anymore?

[jameswnl]

The original output from ec2instance.info already has a field generation which has value either 'current' or 'previous'. So that's how I determine it.

[miq-bot]

Checked commits jameswnl/manageiq-providers-amazon@e5b8c2a~...20fa57d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 4 offenses detected

app/models/manageiq/providers/amazon/instance_types.rb

• ❗ - Line 1329, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 1353, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 1377, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 1401, Col 5 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

[jameswnl]

@miq-bot remove_label wip

[durandom]

I like it :)

@bronaghs I had a quick search for :network_performance and :processor_clock_speed and it seems changing them from numerical to free text has no impact, because they are not really used in the UI.

So, please merge if you like