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

Incorrect source_type in has_many #14206

Merged
merged 2 commits into from Mar 7, 2017
Merged

Conversation

mkanoor
Copy link
Contributor

@mkanoor mkanoor commented Mar 6, 2017

Ansible Tower Services where failing trying to access the
configuration_manager

Links

Steps for Testing/QA

Listed in BZ, provision a service using the Ansible Tower

https://bugzilla.redhat.com/show_bug.cgi?id=1428607

Ansible Tower Services where failing trying to access the
configuration_manager
@mkanoor
Copy link
Contributor Author

mkanoor commented Mar 6, 2017

@jameswnl @bzwei @gmcculloug
Please review

@bzwei
Copy link
Contributor

bzwei commented Mar 6, 2017

@jameswnl This fix will be good for new AnsibleTower services, but for existing ones we need a migration for table service_resources. Any value in column resource_type => ConfigurationScript needs to be migrated to ConfigurationScriptBase.

@mkanoor
Copy link
Contributor Author

mkanoor commented Mar 7, 2017

@bzwei
Please re-review

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2017

Checked commits mkanoor/manageiq@fc768ed~...63dfd5b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 👍

@bzwei
Copy link
Contributor

bzwei commented Mar 7, 2017

Good Travis helps to find a blind spot for us. LGTM

@syncrou
Copy link
Contributor

syncrou commented Mar 7, 2017

👍 - Looks good to me.

@bzwei - Should we create an issue to track the followup migration? - #14206 (comment)

@gmcculloug
Copy link
Member

@mkanoor Want to put together the migration either here or in a separate PR?

@bzwei
Copy link
Contributor

bzwei commented Mar 7, 2017

@jameswnl do you have any pending PR for similar migration? If not, it makes sense to include a migration in the PR.

@jameswnl
Copy link
Contributor

jameswnl commented Mar 7, 2017

I am working on the migration

@jameswnl
Copy link
Contributor

jameswnl commented Mar 7, 2017

Migration PR is up #14222

@gmcculloug gmcculloug merged commit b075710 into ManageIQ:master Mar 7, 2017
@gmcculloug gmcculloug added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 7, 2017
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.

None yet

7 participants