Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … #5446

johnpmitsch · 2015-08-28T16:02:47Z

…database

Opening this up for initial review. This covers adding subscriptions to the database and converting to scoped search on subscriptions/pools.

here is what is left to-do

Issues that need to be resolved

Adding and removing subscriptions from content hosts functioning but with errors.
Autocomplete not showing in UI for activation keys and content hosts
Subscription type attribute isn't loading in the UI
Subscription table in add and list/remove to activation key isn't showing any message when blank

johnpmitsch · 2015-09-02T12:56:18Z

[test]

ehelms · 2015-09-03T12:18:13Z

.../javascripts/bastion_katello/content-hosts/details/views/content-host-add-subscriptions.html

@@ -15,6 +14,7 @@
      </div>
    </div>

+


cfouant · 2015-09-04T20:22:39Z

Much of the UI seems messed up (specifically the subscriptions tab under content hosts). I would fix the technical errors (indentation mostly, a couple missing semicolons - spelled out in the grunt tests); either you or I can then fix the broken javascript tests from there.

johnpmitsch · 2015-09-08T12:53:56Z

@cfouant hey, ya, so new development, both activation keys and content host had to be converted to use nutupane with subscriptions for scoped search to work on those pages, so I am in the middle of that. Also, I am rebasing now, sorry I didn't see that until now. I'll check in with you when you are online this morning and we can discuss further.

cfouant · 2015-09-08T15:17:24Z

app/lib/katello/util/search.rb

+      end
+
+      def self.pulp_backend_search_classes
+        [Katello::PuppetModule]
      end


I think you actually want to remove these two methods now that you PuppetModules have been converted.

Scratch that. Just return an empty array.

@cfouant I don't see a need for either method so I removed it completely and wherever it is called in rake tasks

cfouant · 2015-09-09T17:56:07Z

I am getting a 500 ISE when I try under Red Hat Subscriptions box to search by name:
2015-09-09T17:50:16 [app] [I] Completed 500 Internal Server Error in 224.2ms (Views: 0.3ms | ActiveRecord: 1.8ms)
2015-09-09T17:50:16 [app] [D] With body: {"displayMessage":"Field 'name' not recognized for searching!","errors":["Field 'name' not recognized for searching!"]}

I've replicated this with other given search fields (eg cores)

johnpmitsch · 2015-09-09T19:19:01Z

@cfouant Thanks for catching that, kind of the whole point of this PR :) Just pushed up the fix, it had to do with Subscriptions controller really returning Pools, which confused scoped search. I was able to add a resource_class method to subscriptions controller and now both Subscription and Pool fields are searchable!

cfouant · 2015-09-10T16:16:42Z

test/models/pool_test.rb

+    end
+
+    def test_search_by_account
+      assert_equal Pool.search_for("account = #{@pool_one.account_number}").first, @pool_one


I would rather see this broken down like:

pools = Pool.search_for("account = #{@pool_one.account_number}") assert_equal pools, @pool.one

johnpmitsch · 2015-09-22T21:22:36Z

@jlsherrill updated

ehelms · 2015-09-22T21:57:39Z

I am hitting this while running the upgrade rake task:

ActiveRecord::SubclassNotFound: The single-table inheritance mechanism failed to locate the subclass: 'Katello::MarketingProduct'. This error is raised because the column 'type' is reserved for storing the class in case of inheritance. Please rename this column if you didn't intend it to be used for storing the inheritance class or overwrite Katello::Product.inheritance_column to use another column for that information.
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:143:in `rescue in find_sti_class'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:136:in `find_sti_class'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:62:in `instantiate'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `block (2 levels) in find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `collect!'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `block in find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/explain.rb:41:in `logging_query_plan'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:37:in `find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:171:in `exec_queries'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:160:in `block in to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/explain.rb:34:in `logging_query_plan'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:159:in `to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/bullet-4.14.7/lib/bullet/active_record3x.rb:10:in `to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation/finder_methods.rb:381:in `find_first'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation/finder_methods.rb:122:in `first'
/home/vagrant/katello/app/models/katello/glue/candlepin/subscription.rb:52:in `block in create_product_associations'
/home/vagrant/katello/app/models/katello/glue/candlepin/subscription.rb:49:in `each'
/home/vagrant/katello/app/models/katello/glue/candlepin/subscription.rb:49:in `create_product_associations'
/home/vagrant/katello/app/models/katello/glue/candlepin/subscription.rb:42:in `import_data'
/home/vagrant/katello/app/models/katello/glue/candlepin/candlepin_object.rb:44:in `block in import_all'
/home/vagrant/katello/app/models/katello/glue/candlepin/candlepin_object.rb:42:in `each'
/home/vagrant/katello/app/models/katello/glue/candlepin/candlepin_object.rb:42:in `import_all'
/home/vagrant/katello/lib/katello/tasks/upgrades/2.4/import_subscriptions.rake:8:in `block (4 levels) in <top (required)>'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/bin/ruby_executable_hooks:15:in `eval'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/bin/ruby_executable_hooks

johnpmitsch · 2015-09-23T13:07:54Z

@ehelms I'm having trouble reproducing, but following the traceback leads me to believe the "type" column should be dropped in Katello::Product table as we no longer have a need for single table inheritance?

jlsherrill · 2015-09-23T13:09:33Z

@ehelms i get the same error, was just about ot paste that.

The issue is that you likely need to import a manifest prior to the upgrade to hit the error

jlsherrill · 2015-09-23T13:11:33Z

The fix would be to just delete Products where type => ::Katello::MarketingProduct i believe

johnpmitsch · 2015-09-23T13:12:23Z

@jlsherrill do that when importing subscriptions?

jlsherrill · 2015-09-23T13:13:11Z

@johnpmitsch at the very start of the upgrade script I'd say.

johnpmitsch · 2015-09-23T13:15:18Z

@jlsherrill so if there are existing Products with "MarketingProduct" in the type column, those should be deleted before the updgrade task, because that model no long exists, am I on the right track here?

jlsherrill · 2015-09-23T13:16:41Z

I would say at the start of the upgrade task, not before it, in the migration. @ehelms may have an opinion about this.

johnpmitsch · 2015-09-23T13:36:48Z

@jlsherrill @ehelms updated the rake task to destroy all where type is MarketingProduct

ehelms · 2015-09-23T14:43:47Z

I still get an error below with this update. The issue now is that it's trying to access a class that no longer exists which means you probably have to try re-defining Katello::MarketingProduct in the rake task or do this in the migration (while also defining that class in the migration like we do in other places).

ActiveRecord::SubclassNotFound: The single-table inheritance mechanism failed to locate the subclass: 'Katello::MarketingProduct'. This error is raised because the column 'type' is reserved for storing the class in case of inheritance. Please rename this column if you didn't intend it to be used for storing the inheritance class or overwrite Katello::Product.inheritance_column to use another column for that information.
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:143:in `rescue in find_sti_class'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:136:in `find_sti_class'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/inheritance.rb:62:in `instantiate'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `block (2 levels) in find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `collect!'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:38:in `block in find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/explain.rb:41:in `logging_query_plan'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/querying.rb:37:in `find_by_sql'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:171:in `exec_queries'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:160:in `block in to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/explain.rb:34:in `logging_query_plan'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:159:in `to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/bullet-4.14.7/lib/bullet/active_record3x.rb:10:in `to_a'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/relation.rb:355:in `destroy_all'
/home/vagrant/katello/lib/katello/tasks/upgrades/2.4/import_subscriptions.rake:7:in `block (4 levels) in <top (required)>'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/bin/ruby_executable_hooks:15:in `eval'
/home/vagrant/.rvm/gems/ruby-1.9.3-p448/bin/ruby_executable_hooks:15:in `<main>'
Tasks: TOP => katello:upgrades:2.4:import_subscriptions

johnpmitsch · 2015-09-23T14:45:33Z

@ehelms is there a good example I can use where we do that in other places?

jlsherrill · 2015-09-23T15:06:03Z

You may just do a delete_all instead of a destroy all. That shouldn't require the class being loaded i don't think.

ehelms · 2015-09-23T16:16:26Z

I also did a grep of 'Marketing' in the app/ directory and found some instances of it.

@jlsherrill Should we drop the inheritance that is still present on the Product model

jlsherrill · 2015-09-23T16:29:04Z

@ehelms by drop the inheritance, i assume you mean the 'type' field? I'd say yeah, that'd be smart to do.

jlsherrill · 2015-09-23T16:45:43Z

and if we do that, the marking product deletion would need to go in the migration that dropped that field.

ehelms · 2015-09-23T16:47:31Z

Yes, that is what I meant (to drop the type column all together).

johnpmitsch · 2015-09-23T19:55:49Z

@ehelms @jlsherrill updated to remove "type" column and include the class in the migration file. I tested by checking out master branch, reseting, import a manifest, switch to subscriptions branch, migrate, run upgrade script and all seemed to run no problem.

ehelms · 2015-09-24T12:47:25Z

db/migrate/20150908222711_drop_marketing_engineering_products.rb

+      t.integer "engineering_product_id"
+    end
+
+    add_column :katello_products, :type


This throws an error on rollback:

StandardError: An error has occurred, this and all later migrations canceled: wrong number of arguments (2 for 3)/home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/connection_adapters/postgresql_adapter.rb:1017:in `add_column' /home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/migration.rb:466:in `block in method_missing' /home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/migration.rb:438:in `block in say_with_time' /home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/migration.rb:438:in `say_with_time' /home/vagrant/.rvm/gems/ruby-1.9.3-p448/gems/activerecord-3.2.21/lib/active_record/migration.rb:458:in `method_missing' /home/vagrant/katello/db/migrate/20150908222711_drop_marketing_engineering_products.rb:18:in `down'

@ehelms updated add_column to include the missing argument

ehelms · 2015-09-24T13:41:42Z

Tested and ACK for me, final say is on @jlsherrill

jlsherrill · 2015-09-24T13:54:10Z

db/migrate/20150908222711_drop_marketing_engineering_products.rb

+    drop_table :katello_marketing_engineering_products
+
+    remove_column :katello_products, :type
+  end


I may have missed it, but i don't see you removing the old marketing products prior to dropping the 'type' column?

@jlsherrill updated

…to the database

jlsherrill · 2015-09-28T14:34:33Z

ACK thanks @johnpmitsch !

crossing fingers that we didn't miss any issues :)

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the …

theforeman-bot added Needs testing Not yet reviewed labels Aug 28, 2015

johnpmitsch force-pushed the subscriptions branch 2 times, most recently from 01550ad to 285462f Compare September 1, 2015 22:16

ehelms reviewed Sep 3, 2015
View reviewed changes

johnpmitsch force-pushed the subscriptions branch from f3d9575 to 88bad85 Compare September 8, 2015 12:57

cfouant reviewed Sep 8, 2015
View reviewed changes

johnpmitsch force-pushed the subscriptions branch from 853b02b to 1d0bec7 Compare September 9, 2015 16:24

johnpmitsch force-pushed the subscriptions branch 3 times, most recently from cade5f0 to 87c8429 Compare September 10, 2015 16:02

cfouant reviewed Sep 10, 2015
View reviewed changes

johnpmitsch changed the title ~~[DO NOT MERGE] Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the …~~ Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … Sep 10, 2015

johnpmitsch force-pushed the subscriptions branch from 58b672e to efd2ee3 Compare September 11, 2015 13:33

johnpmitsch force-pushed the subscriptions branch from 9193a78 to a48f18e Compare September 23, 2015 13:34

johnpmitsch force-pushed the subscriptions branch 2 times, most recently from 52a55f4 to cd5a22f Compare September 23, 2015 19:29

ehelms reviewed Sep 24, 2015
View reviewed changes

johnpmitsch force-pushed the subscriptions branch from cd5a22f to f4962f7 Compare September 24, 2015 13:05

jlsherrill reviewed Sep 24, 2015
View reviewed changes

Fixes Katello#7891 - Convert Pools/Subscriptions from Elastic Search …

6400e13

…to the database

johnpmitsch force-pushed the subscriptions branch from f4962f7 to 6400e13 Compare September 24, 2015 16:37

jlsherrill added a commit that referenced this pull request Sep 28, 2015

Merge pull request #5446 from johnpmitsch/subscriptions

0680624

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the …

jlsherrill merged commit 0680624 into Katello:master Sep 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … #5446

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … #5446

johnpmitsch commented Aug 28, 2015

johnpmitsch commented Sep 2, 2015

ehelms Sep 3, 2015

cfouant commented Sep 4, 2015

johnpmitsch commented Sep 8, 2015

cfouant Sep 8, 2015

cfouant Sep 8, 2015

johnpmitsch Sep 8, 2015

cfouant commented Sep 9, 2015

johnpmitsch commented Sep 9, 2015

cfouant Sep 10, 2015

johnpmitsch commented Sep 22, 2015

ehelms commented Sep 22, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

ehelms commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

ehelms commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

ehelms commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

ehelms Sep 24, 2015

johnpmitsch Sep 24, 2015

ehelms commented Sep 24, 2015

jlsherrill Sep 24, 2015

johnpmitsch Sep 24, 2015

jlsherrill commented Sep 28, 2015

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … #5446

Fixes #7891 - Convert Pools/Subscriptions from Elastic Search to the … #5446

Conversation

johnpmitsch commented Aug 28, 2015

johnpmitsch commented Sep 2, 2015

Choose a reason for hiding this comment

cfouant commented Sep 4, 2015

johnpmitsch commented Sep 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfouant commented Sep 9, 2015

johnpmitsch commented Sep 9, 2015

Choose a reason for hiding this comment

johnpmitsch commented Sep 22, 2015

ehelms commented Sep 22, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

ehelms commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

ehelms commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

jlsherrill commented Sep 23, 2015

ehelms commented Sep 23, 2015

johnpmitsch commented Sep 23, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehelms commented Sep 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlsherrill commented Sep 28, 2015