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

Add the resource name to the task action for conversion hosts #18525

Merged
merged 3 commits into from
Mar 7, 2019
Merged

Add the resource name to the task action for conversion hosts #18525

merged 3 commits into from
Mar 7, 2019

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Mar 6, 2019

Because there's no direct association between conversion hosts and tasks we're relying on the task description to suss them out. This PR merely modifies the task description so that it includes the resource name as well.

This would save the UI team from having to query /api/vms and /api/hosts with the IDs from the tasks just to display a name on those list items.

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

@djberg96
Copy link
Contributor Author

djberg96 commented Mar 6, 2019

@miq-bot add_label transformation, enhancement, hammer/yes

@djberg96
Copy link
Contributor Author

djberg96 commented Mar 6, 2019

@mturley Look ok?

@mturley
Copy link
Contributor

mturley commented Mar 6, 2019

Looks perfect, thanks @djberg96! That was fast.

@mturley
Copy link
Contributor

mturley commented Mar 6, 2019

Actually, it's fine either way, but maybe we should add a space after id: to make them all consistent?

@djberg96
Copy link
Contributor Author

djberg96 commented Mar 6, 2019

@mturley done!

@miq-bot
Copy link
Member

miq-bot commented Mar 6, 2019

Checked commits https://github.com/djberg96/manageiq/compare/fb38fba360f5b8e12e277537e05cdd0245e4b05e~...40f308b1d8ef61f08366e9db63e94995864bae95 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. 🏆

@mturley
Copy link
Contributor

mturley commented Mar 6, 2019

We can probably add this to the conversion host enablement feature BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1622728

@agrare agrare self-assigned this Mar 7, 2019
Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

No issue with this, I'm not a fan of how it is going to be used but don't see a better option right now.

We should investigate doing a proper polymorphic assoc between miq_queue and other instances.

@agrare agrare merged commit bcc55f9 into ManageIQ:master Mar 7, 2019
@agrare agrare added this to the Sprint 107 Ending Mar 18, 2019 milestone Mar 7, 2019
@mturley
Copy link
Contributor

mturley commented Mar 7, 2019

Agreed @agrare, let's revisit this when we have time. I'm tracking that on the v2v side in ManageIQ/manageiq-v2v#895

simaishi pushed a commit that referenced this pull request Apr 22, 2019
Add the resource name to the task action for conversion hosts

(cherry picked from commit bcc55f9)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702034
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit aa4fafc0c560aea5dbdda3b16531c9bed1d8486c
Author: Adam Grare <agrare@redhat.com>
Date:   Thu Mar 7 11:01:56 2019 -0500

    Merge pull request #18525 from djberg96/conversion_host_enable
    
    Add the resource name to the task action for conversion hosts
    
    (cherry picked from commit bcc55f9a6d0bbedbea52582ff4783fb86830650e)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1702034

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.

5 participants