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

STI not working correctly with subclasses across providers #14154

Closed
durandom opened this issue Mar 3, 2017 · 6 comments
Closed

STI not working correctly with subclasses across providers #14154

durandom opened this issue Mar 3, 2017 · 6 comments
Assignees

Comments

@durandom
Copy link
Member

durandom commented Mar 3, 2017

[1] pry(main)> Authentication.subclasses.include?(ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential)
=> false

But it should be true?!

Here Rails compares with > which breaks it.

Whats even the difference between subclasses and descendants ?

My Problem is

 1) ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Refresher will perform a full refresh
     Failure/Error: raise _("%{class_name} is not a subclass of %{name}") % {:class_name => klass.name, :name => name}

     RuntimeError:
       ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential is not a subclass of
     # ./app/models/mixins/new_with_type_sti_mixin.rb:11:in `new'
     # ./app/models/authentication.rb:7:in `new'
     # ./app/models/manager_refresh/save_collection/helper.rb:100:in `create_record!'
[19] pry(main)> Authentication.new(:type => 'ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential')
RuntimeError: ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential is not a subclass of AuthUseridPassword
from /Users/hild/src/manageiq/app/models/mixins/new_with_type_sti_mixin.rb:11:in `new'
[20] pry(main)> Authentication.descendants.include?(ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential)
=> true

Maybe this can help?
rails/rails#19531

@miq-bot assign @chrisarcand
@miq-bot add_label embedded_ansible_madness

cc @blomquisg

@miq-bot
Copy link
Member

miq-bot commented Mar 3, 2017

@durandom Cannot apply the following label because they are not recognized: embedded_ansible_madness

@durandom
Copy link
Member Author

durandom commented Mar 3, 2017

please note, it's not a blocking problem yet, because I can

ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Authentication.new(:type => 'ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential')

but...

@chrisarcand
Copy link
Member

chrisarcand commented Mar 3, 2017

Whats even the difference between subclasses and descendants ?

subclasses are direct subclasses
descendants are what you think they are, including subclasses of subclasses.

https://github.com/rails/rails/blob/d47b90e76c989a8f0ca32c140e9b7220da0e5edc/activesupport/lib/active_support/core_ext/class/subclasses.rb

But it should be true?!

Nope, the result is what I would expect :)

@durandom
Copy link
Member Author

durandom commented Mar 3, 2017

@chrisarcand yeah, thanks. Now the difference makes sense...

But still, why cant I do

[19] pry(main)> Authentication.new(:type => 'ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential')
RuntimeError: ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential is not a subclass of AuthUseridPassword
from /Users/hild/src/manageiq/app/models/mixins/new_with_type_sti_mixin.rb:11:in `new'

Shouldnt the mixin check descendants?!

@chrisarcand
Copy link
Member

It does; Authentication calls AuthUseridPassword here which causes your error. I'm not sure why this is architected how it is but are you sure you're using this API as it's intended to be used? cc/ @Fryguy

@durandom
Copy link
Member Author

durandom commented Mar 3, 2017

Ah, damn, now that you say, I remember I stumbled across this when doing the Authentication / Credential stuff...

sorry for stirring up stuff.

@durandom durandom closed this as completed Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants