Add conversion_host_id to miq_request_task #281

ghost · 2018-10-01T13:22:51Z

The ServiceTemplateTransformationPlanTask class relies on a conversion host to actually run the migration. The conversion host is modeled in ConversionHost class. From a database point of view, the ServiceTemplateTransformationPlanTask class is stored in miq_request_tasks table, because it's a sub-sub-class of MiqRequestTask. This PR adds a conversion_host_id column to miq_request_tasks to set the relationship.

Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1634029

carbonin · 2018-10-01T14:27:55Z

It looks like this information is currently being stored in the ServiceTemplateTransformationPlanTask#options column. I think this PR should also include a data migration to remove that value and put it in the new column for all previous tasks.

ghost · 2018-10-01T14:44:39Z

@carbonin That's even more complex. Today, the information stored in options[:transformation_host_id] is the id of a host (class Host). The PR stores the conversion host (class ConversionHost) id. A conversion host has a resource which is either a Host or a Vm, stored as resource_type. So current migration should create a ConversionHost with a resource relationship to a resource_type object with id resource_id, if it doesn't exist. Then the conversion_host_id can be set.

@agrare what are the possible values for resource_type ? Host and Vm ?

agrare · 2018-10-01T15:06:25Z

@fdupont-redhat if options[:transformation_host_id] can only ever contain a Host object then this should create a ConversionHost record for every distinct options[:transformation_host_id] and then conversion_host.resource = Host.find(options[:transformation_host_id]).

It is a little more complicated than just Host or Vm because the hosts will be STI'd e.g. ManageIQ::Providers::Redhat::InfraManager::Host.

ghost · 2018-10-01T21:38:42Z

@miq-bot add-label gaprindashvili/no, schema, transformation

miq-bot · 2018-10-02T06:41:32Z

@fdupont-redhat 'agrare, carbonin' is an invalid reviewer, ignoring...

ghost · 2018-10-02T08:07:20Z

@miq-bot add-reviewer agrare
@miq-bot add-reviewer carbonin

carbonin · 2018-10-02T12:43:22Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+  def up
+    add_column :miq_request_tasks, :conversion_host_id, :bigint
+
+    ServiceTemplateTransformationPlanTask.all.reject { |task| task.options[:transformation_host_id].nil? }.each do |task|


A few things here ...
First, I don't think this query will work the way you're intending it to. STI is disabled so I think you would be better off using the base class and specifying the type in a where clause.

Also, instead of using reject here, can we make this a guard clause in the each block? That will simplify this and prevent us from iterating through the list of tasks twice.

So something like this:

MiqRequestTask.where(:type => "ServiceTemplateTransformationPlanTask").each do |task| next unless task.options[:transformation_host_id] ... end

Thanks for the tip. Updated.

carbonin · 2018-10-02T12:46:05Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+  end
+
+  def down
+    ServiceTemplateTransformationPlanTask.all.select { |task| task.conversion_host.present? }.each do |task|


Same here, use the base class and you don't need to loop through the tasks twice.

carbonin · 2018-10-02T12:47:49Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+
+  class Host < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+    has_many :tags, :class_name => "AddConversionHostIdToMiqRequestTasks::Tag"


Why is this relation needed?

Remnant of tests. Removed.

carbonin · 2018-10-02T12:48:36Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+
+  class ConversionHost < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+    has_many :service_template_transformation_plan_tasks, :class_name => "AddConversionHostIdToMiqRequestTasks::MiqRequestTask"


This relation isn't being used in the migration, why define it here?

Correct. Removed.

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

carbonin · 2018-10-02T12:55:22Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+    belongs_to :conversion_host, :class_name => "AddConversionHostIdToMiqRequestTasks::ConversionHost"
+  end
+
+  class ServiceTemplateTransformationPlanTask < MiqRequestTask


I think this can be removed ... See my other comments about STI and using the base class for querying.

carbonin · 2018-10-02T12:57:31Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+    add_column :miq_request_tasks, :conversion_host_id, :bigint
+
+    ServiceTemplateTransformationPlanTask.all.reject { |task| task.options[:transformation_host_id].nil? }.each do |task|
+      host = Host.find_by(:id => task.options[:transformation_host_id])


I think this can use find rather than find_by. Also this entire block would be way easier to read if task.options[:transformation_host_id] was stored in a local variable (ideally before the guard clause).

agrare · 2018-10-02T13:00:28Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+
+    ServiceTemplateTransformationPlanTask.all.reject { |task| task.options[:transformation_host_id].nil? }.each do |task|
+      host = Host.find_by(:id => task.options[:transformation_host_id])
+      task.conversion_host = ConversionHost.where(:id => task.options[:transformation_host_id]).first_or_create do |ch|


find_or_create_by! would be better over .where(:id => ...).first_or_create

Also, if options[:transformation_host_id] is a reference to the Host record id then it can't also be a reference to the ConversionHost id.

This where just looks wrong.

Oh yeah good catch, @fdupont-redhat did you mean to ConversionHost.where(:id => host.conversion_host.try(:id)) or something?

Changed for find_or_create_by!.
The where was wrong. Now it checks the resource_id attribute against the host_id.

You should check for the type also then, just checking for the id and not the type

agrare · 2018-10-02T13:01:38Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+    add_column :miq_request_tasks, :conversion_host_id, :bigint
+
+    ServiceTemplateTransformationPlanTask.all.reject { |task| task.options[:transformation_host_id].nil? }.each do |task|
+      host = Host.find_by(:id => task.options[:transformation_host_id])


Should probably throw in a next if host.nil? since you're using host.name, host.type, ... below just in case the host was deleted and the task options weren't updated which given they aren't kept in sync by refresh is a distinct possibility.

Ah, if that's a possibility @agrare then we should also keep the find_by (find raises if the record doesn't exist).

Added test on host.nil?

agrare · 2018-10-02T13:02:28Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+      task.conversion_host = ConversionHost.where(:id => task.options[:transformation_host_id]).first_or_create do |ch|
+        ch.name                     = host.name
+        ch.resource_type            = host.type
+        ch.resource_id              = host.id


Should be able to just do ch.resource = host here

When using find_or_create_by! the resource_id attribute is already set, so now I simply add the resource_type.

agrare · 2018-10-02T13:03:50Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+        ch.name                     = host.name
+        ch.resource_type            = host.type
+        ch.resource_id              = host.id
+        ch.address                  = host.ipaddress


If we're going to delegate ipaddress to the resource per #242 (comment) then we should leave this blank in the conversion_host record. That way if the host ip address changes it will be automatically picked up.

Good catch.

agrare · 2018-10-02T13:10:48Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+    it "creates conversion host" do
+      task = task_stub.create!
+      host = host_stub.create!
+      conversion_host = conversion_host_stub.create!(


You can just do .create!(:resource => host) no need to split up the id/type

Why are you creating a ConversionHost in a spec that should be testing that a conversion host is created?

Good question @carbonin. Probably, because I'm dumb 😄
Removed and spec updated.

@agrare It fails if I use :resource => host. The error message is:

ActiveModel::UnknownAttributeError: unknown attribute 'resource' for AddConversionHostIdToMiqRequestTasks::ConversionHost.

You probably need to add the relation to the stubbed model

agrare · 2018-10-02T13:11:44Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+        :resource_id   => host.id,
+        :resource_type => host.type
+      )
+      task.options = { :transformation_host_id => conversion_host.id }


You can pass :options => {:trans....} into the create!() and then you don't need the extra save

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

agrare · 2018-10-02T13:12:48Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+        :resource_type => host.type
+      )
+      conversion_host.save
+      task.conversion_host = conversion_host


Likewise just pass :conversion_host in to the .create!

agrare · 2018-10-02T13:13:20Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+      migrate
+
+      task.reload
+      expect(task.attributes).not_to include('conversion_host_id')


No need to check that ActiveRecord deleted the column, if that is broken we have bigger issues

carbonin · 2018-10-02T13:21:38Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+        :resource_id   => host.id,
+        :resource_type => host.type
+      )
+      task.options = { :transformation_host_id => conversion_host.id }


Also, shouldn't this be a Host reference, not a conversion_host reference?

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

agrare · 2018-10-02T15:21:09Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+
+    MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').each do |task|
+      host_id = task.options[:transformation_host_id]
+      if host_id


I know it is a style thing but I usually prefer next if host_id.nil? over if host_id and then everything else indented. Reminds me of the java only one return style which brings back bad memories 😄

Me too. But code climate complains about cognitive complexity when using next if. Hadn't met that error before and I use quite some next if or next unless in Automate.

Don't worry about code climate, ha and rubocop complained for not using next to skip
Can't win :)

…d from task.options

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

agrare · 2018-10-02T17:15:15Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+      task.options[:transformation_host_id] = task.conversion_host.resource.id
+      task.save!
+      task.conversion_host.id
+    end


Should .uniq also since there will be many tasks with the same conversion host. You can do end.uniq.compact or ConversionHost.destroy(conversion_host_ids.uniq.compact) your call

agrare · 2018-10-02T17:20:43Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

@@ -35,11 +34,12 @@ def up

  def down
    conversion_host_ids = MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').map do |task|
+      return if task.conversion_host.nil?


Should be next if

agrare · 2018-10-02T17:21:46Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+      end
+      task.save!
+    end
+  end


the #up method looks good to me now

agrare

LGTM

@carbonin can you do another once-over?

carbonin

Looking much better, just a few comments on the specs.

carbonin · 2018-10-02T17:44:38Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+  let(:conversion_host_stub) { migration_stub(:ConversionHost) }
+
+  migration_context :up do
+    it "when host doesn't exist" do


Nit, but it would be great if this wrote out what we were testing for.

For example, it "doesn't set the conversion host when the host object doesn't exist"

carbonin · 2018-10-02T17:45:22Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+      expect(task.conversion_host).to be_nil
+    end
+
+    it "when host exist" do


Same here, it "references the existing Host in the ConversionHost record"

carbonin · 2018-10-02T17:46:20Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+      task.reload
+
+      expect(task.options).to eq(:dummy_key => 'dummy_value')
+      expect(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource_id => conversion_host_id)).to be_nil


I think this should be conversion_host_stub.find_by... right?

carbonin · 2018-10-02T17:46:34Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+      task.reload
+
+      expect(task.options).to eq(:dummy_key => 'dummy_value')
+      expect(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource => host)).not_to be_nil


Same here, I think this should use the stub.

carbonin · 2018-10-02T17:46:40Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+
+      expect(task.options).to eq(:dummy_key => 'dummy_value')
+      expect(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource => host)).not_to be_nil
+      expect(task.conversion_host).to eq(AddConversionHostIdToMiqRequestTasks::ConversionHost.find_by(:resource => host))


miq-bot · 2018-10-02T17:58:49Z

Checked commits fabiendupont/manageiq-schema@659be65~...7c3eb63 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

bdunne

Aside from the code comments, the BZ is marked with a Target release of 5.10 (based on hammer branch) and we've already passed the schema freeze for hammer. I hope this PR isn't expected to be backported to hammer.

bdunne · 2018-10-02T18:03:48Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+
+  class ConversionHost < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+    belongs_to :resource, :polymorphic => true


I don't think you can use polymorphic relations with the stub models. If you do, you'll get something like: :resource_type => MigrationClass::StubModel which won't be found in production.

Yeah you're right I thought in production it would use the type from the record but appears not

Okay, who wants to take on changing this? Or should we write a follow up migration to alter all the broken resources to the "production ready" name?

I can, should be able to just edit this migration?

If we're going to update the .where to .in_my_region.find_each anyway

I think Jason would say we need a new migration since developers may have already run this one...

Okay #284 data migration to fix the resource_type

bdunne · 2018-10-02T18:22:52Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+  def up
+    add_column :miq_request_tasks, :conversion_host_id, :bigint
+
+    MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').each do |task|


Usually we prefer find_each to load records in batches.

Also, this should probably be scoped with in_my_region to avoid running the migration on more records than necessary.

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

bdunne · 2018-10-02T18:33:14Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+      next unless host_id
+      host = Host.find_by(:id => host_id)
+      if host.present?
+        task.conversion_host = ConversionHost.find_or_create_by!(:resource => host) do |ch|


ConversionHost.find_or_initialize_by(:resource_type => "Host", :resource_id => host.id).update_attributes!(.....)

I don't see why we should use find_or_initialize_by + update_attributes over what's here, but yeah it does look like this will break because the host class will be incorrect.

bdunne · 2018-10-02T18:37:56Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+  def down
+    conversion_host_ids = MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').map do |task|
+      next if task.conversion_host.nil?
+      task.options[:transformation_host_id] = task.conversion_host.resource.id


Maybe task.conversion_host.resource_id to avoid loading resource (which I think would fail anyway)?

bdunne · 2018-10-02T18:40:58Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+        :type    => 'ServiceTemplateTransformationPlanTask',
+        :options => { :dummy_key => 'dummy_value', :transformation_host_id => conversion_host_id }
+      )
+      host.destroy!


You should be able to generate a record id (in the current region) without creating and destroying a record

I'm not sure how you could do that without querying the sequences. I think it would have been nice to create the host in a before block, but other than that I think this is fine.

bdunne · 2018-10-02T18:42:12Z

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

+  end
+
+  def down
+    conversion_host_ids = MiqRequestTask.where(:type => 'ServiceTemplateTransformationPlanTask').map do |task|


This should also be scoped with in_my_region

bdunne · 2018-10-02T18:46:47Z

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb

+      task.reload
+
+      expect(task.options).to eq(:dummy_key => 'dummy_value')
+      expect(conversion_host_stub.find_by(:resource => host)).not_to be_nil


shouldn't this eq(something)?

Fix an invalid polymorphic resource_type from ManageIQ#281 which resulted in the type being "AddConversionHostIdToMiqRequestTasks::Host" instead of "Host".

…d_to_transformation_task Add conversion_host_id to miq_request_task (cherry picked from commit eb7e077) https://bugzilla.redhat.com/show_bug.cgi?id=1634029

simaishi · 2018-10-03T14:19:08Z

Backport approved by Engineering/QE.

Hammer backport details:

$ git log -1
commit 58bd0c05dd61a8f5597f9d49f1e4fedb39cb88ec
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Tue Oct 2 14:02:07 2018 -0400

    Merge pull request #281 from fdupont-redhat/v2v_add_conversion_host_id_to_transformation_task
    
    Add conversion_host_id to miq_request_task
    
    (cherry picked from commit eb7e077f462b9ab5e1c652f8220a34d22e4c8633)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1634029

ghost mentioned this pull request Oct 1, 2018

Add methods to conversion_host to build virt-v2v wrapper options ManageIQ/manageiq#18033

Merged

Add conversion host relationship to task

659be65

ghost force-pushed the v2v_add_conversion_host_id_to_transformation_task branch from 3713049 to 659be65 Compare October 1, 2018 21:35

miq-bot added gaprindashvili/no schema labels Oct 1, 2018

miq-bot requested review from agrare and carbonin October 2, 2018 08:07

carbonin suggested changes Oct 2, 2018

View reviewed changes

agrare reviewed Oct 2, 2018

View reviewed changes

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb Outdated Show resolved Hide resolved

agrare reviewed Oct 2, 2018

View reviewed changes

carbonin reviewed Oct 2, 2018

View reviewed changes

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb Show resolved Hide resolved

bdunne added the hammer/no label Oct 2, 2018

ghost force-pushed the v2v_add_conversion_host_id_to_transformation_task branch from eb6b3cb to 7d45a4e Compare October 2, 2018 15:20

agrare reviewed Oct 2, 2018

View reviewed changes

ghost force-pushed the v2v_add_conversion_host_id_to_transformation_task branch from 7d45a4e to 04ca79a Compare October 2, 2018 15:30

Replace host_id by host stub .id. Always delete transformation_host_i…

f99b040

…d from task.options

agrare reviewed Oct 2, 2018

View reviewed changes

spec/migrations/20181001131632_add_conversion_host_id_to_miq_request_tasks_spec.rb Show resolved Hide resolved

agrare reviewed Oct 2, 2018

View reviewed changes

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb Show resolved Hide resolved

agrare reviewed Oct 2, 2018

View reviewed changes

Save some CPU cycles

6f67585

agrare reviewed Oct 2, 2018

View reviewed changes

db/migrate/20181001131632_add_conversion_host_id_to_miq_request_tasks.rb

end

task.save!

end

end

Copy link

Member

agrare Oct 2, 2018

Choose a reason for hiding this comment

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

the #up method looks good to me now

Fix return with next

d9bfb4d

agrare approved these changes Oct 2, 2018

View reviewed changes

carbonin reviewed Oct 2, 2018

View reviewed changes

Address review comments about specs

7c3eb63

carbonin approved these changes Oct 2, 2018

View reviewed changes

carbonin self-assigned this Oct 2, 2018

carbonin merged commit eb7e077 into ManageIQ:master Oct 2, 2018

carbonin added this to the Sprint 96 Ending Oct 8, 2018 milestone Oct 2, 2018

bdunne reviewed Oct 2, 2018

View reviewed changes

agrare mentioned this pull request Oct 2, 2018

Fix invalid conversion host resource type #284

Merged

simaishi added hammer/backported and removed hammer/no labels Oct 3, 2018

ghost mentioned this pull request Oct 3, 2018

expose auth_key to automate for AuthTokens ManageIQ/manageiq-automation_engine#239

Closed

ghost deleted the v2v_add_conversion_host_id_to_transformation_task branch October 16, 2018 09:45

Fryguy added v2v and removed transformation labels Feb 28, 2020

Add conversion_host_id to miq_request_task #281

Add conversion_host_id to miq_request_task #281

Conversation

ghost commented Oct 1, 2018 • edited by ghost Loading

carbonin commented Oct 1, 2018

ghost commented Oct 1, 2018

agrare commented Oct 1, 2018

ghost commented Oct 1, 2018 • edited by ghost Loading

miq-bot commented Oct 2, 2018

ghost commented Oct 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrare Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrare left a comment

Choose a reason for hiding this comment

carbonin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Oct 2, 2018

bdunne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Oct 1, 2018 •

edited by ghost

Loading

ghost commented Oct 1, 2018 •

edited by ghost

Loading

agrare Oct 2, 2018 •

edited

Loading