Native viewer #6574

gekorob · 2019-12-24T16:40:04Z

Show native viewer button for redhat providers and allow
connection file to be downloaded after requiring it via task

related to BZ:
https://bugzilla.redhat.com/show_bug.cgi?id=1451594

Depends on:

gekorob · 2019-12-24T16:40:23Z

@miq_bot add_label wip

skateman · 2019-12-24T21:33:02Z

app/helpers/application_helper/button/vm_native_viewer_console.rb

+
+  def disabled?
+    begin
+      @record.validate_native_viewer_support


Is this calling something from the provider's API, because then it can heavily affect the performance. cc @agrare

Oh sorry, didn't see the wip 😕

@skateman this call is similar to the one that is doing vmware VRMC button, it's only to test if ems is not nil and state is 'on'. No call to external provider.

please feel free to suggest modifications

BTW Merry Christmas 'cause I realized that midnight has passed

Okkay, I'm gonna dig into these. Happy new year!

Many thanks and Happy new year also to you and the team

skateman · 2019-12-24T21:33:34Z

@miq-bot add_label wip

skateman · 2020-01-03T08:59:09Z

What about naming it a more universal Native Console instead of Native Viewer Console? That way we might be able to extend it to other providers in the future without introducing provider-specific features. Also it's shorter 😉 what do you think?

gekorob · 2020-01-03T09:08:37Z

Hi @skateman, yes absolutely. I have to admit that I tried several namings, and before it was Native Console. So yes, no problem to change to that. When I'll back to work I'll change to NativeConsole

gekorob · 2020-01-03T09:15:30Z

@skateman, may I ask also to check where I manage the task_complete?
https://github.com/gekorob/manageiq-ui-classic/blob/4cc30e5b47cce2656e2c897809d1a3a8f4f2fadf/app/controllers/vm_remote.rb#L134-L138

I'm passing the task_id to the method that is executing the send_data
https://github.com/gekorob/manageiq-ui-classic/blob/4cc30e5b47cce2656e2c897809d1a3a8f4f2fadf/app/controllers/vm_remote.rb#L28-L34

The price is to execute two times a MiqTask.find but I wanted to avoid passing the Base64 encoded console file as parameter.
Is there another way to do that?

Show native viewer button for redhat providers and allow connection file (usually named console.vv) to be dowloaded after requiring via task

miq-bot · 2020-01-07T16:41:14Z

Checked commit gekorob@4503228 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

gekorob · 2020-01-07T16:54:38Z

@skateman I changed from Native Viewer Console to Native Console as you suggested. Also changed in manageiq core and in the provider.

gekorob · 2020-01-07T16:54:52Z

@miq-bot remove_label wip

skateman · 2020-01-08T09:33:12Z

app/controllers/vm_remote.rb

+  end
+
+  def native_console_task_complete
+    miq_task = MiqTask.find(params[:task_id])


Do we really need to search for the MiqTask here, as far as I see, it's just "validation" which could be done in the next step when opening the window.

Yes it's only to test if the task has results. You're right I'll try to move it in the launch_native_console so we can remove the first find.

skateman · 2020-01-08T09:34:36Z

app/helpers/application_helper/toolbar/x_vm_center.rb

+        ),
+        button(
+          :vm_native_console,
+          'pficon pficon-screen fa-lg',


@epwinchell maybe we need a different icon for this, your call.

skateman · 2020-01-08T09:36:12Z

For the first look, the code looks fine, with some minor things. Maybe we want to extend this into the Service UI, cc @martinpovolny and @himdel if we want that (but my guess is yes). I can help you with that.

gekorob · 2020-01-08T10:40:51Z

@skateman do you have suggestions for the i18n messages?
I think I have to add it in the core PR (ManageIQ/manageiq#19675).
Is it right?

mzazrivec · 2020-01-08T12:08:58Z

do you have suggestions for the i18n messages? I think I have to add it in the core PR

Which messages are we talking about here?

gekorob · 2020-01-08T12:24:16Z

I was thinking at strings related to the button:

        button(
          :vm_native_console,
          'pficon pficon-screen fa-lg',
          N_('Open a native console for this VM'),
          N_('Native Console'),
          :keepSpinner => true,
          :url         => "native_console",
          :klass       => ApplicationHelper::Button::VmNativeConsole
        ),

'cause Native Console didn't exists before as a button.
I'm quite new to the ui part, sorry

skateman · 2020-01-08T12:29:47Z

AFAIK you don't have to do more than you did, the N_ call should take care of the translation. But @mzazrivec can tell you more about this.

gekorob · 2020-01-08T12:50:20Z

Great thx a lot, so I'll leave it like this

epwinchell · 2020-01-08T17:18:42Z

@gekorob Could you post a screenshot? Thanks

martinpovolny · 2020-01-08T18:07:32Z

app/helpers/application_helper/button/vm_native_console.rb

+
+  def disabled?
+    begin
+      @record.validate_native_console_support


Can you confirm that thisvalidate_native_console_support only talks to the database? (it does not do any time consuming operation and does to try to access the VM or talk to a provider).

Thx!

Hi @martinpovolny, yes, it's testing only if the ext_management_system is nil and the state is on.
https://github.com/ManageIQ/manageiq-providers-ovirt/blob/c544028e4aea14910c0a9ebacb02107dd4156f98/app/models/manageiq/providers/redhat/infra_manager/vm/remote_console.rb#L43-L49

gekorob · 2020-01-08T18:30:39Z

Sure @epwinchell, in the drop down list, native console has the same icon of the other console. I thought it was ok, but if you have another one I can refer to that.

Tell me if is it ok for you.
Thx a lot

epwinchell

@gekorob Thanks. It looks good. We'll keep it simple.

skateman · 2020-01-09T06:59:18Z

Gonna test this

skateman

skateman · 2020-01-11T22:55:23Z

@martinpovolny there is a pending core PR

simaishi · 2020-10-06T16:02:20Z

Backported to ivanchuk via #7372

skateman reviewed Dec 24, 2019

View reviewed changes

miq-bot changed the title ~~Native viewer~~ [WIP] Native viewer Dec 24, 2019

miq-bot added the wip label Dec 24, 2019

Native viewer

4503228

Show native viewer button for redhat providers and allow connection file (usually named console.vv) to be dowloaded after requiring via task

gekorob force-pushed the rfe/native_viewer branch from 4cc30e5 to 4503228 Compare January 7, 2020 16:37

miq-bot changed the title ~~[WIP] Native viewer~~ Native viewer Jan 7, 2020

miq-bot removed the wip label Jan 7, 2020

gekorob mentioned this pull request Jan 7, 2020

Native viewer support ManageIQ/manageiq-providers-ovirt#452

Merged

skateman reviewed Jan 8, 2020

View reviewed changes

skateman mentioned this pull request Jan 8, 2020

RHV Native viewer ManageIQ/manageiq#19675

Merged

miq-bot requested a review from epwinchell January 8, 2020 15:52

martinpovolny reviewed Jan 8, 2020

View reviewed changes

epwinchell approved these changes Jan 8, 2020

View reviewed changes

skateman approved these changes Jan 9, 2020

View reviewed changes

martinpovolny merged commit 8e67de8 into ManageIQ:master Jan 11, 2020

martinpovolny self-assigned this Jan 11, 2020

martinpovolny added compute/infrastructure enhancement labels Jan 11, 2020

martinpovolny added this to the Sprint 128 Ending Jan 20, 2020 milestone Jan 11, 2020

mzazrivec mentioned this pull request Oct 1, 2020

[IVANCHUK] Support for ovirt native console #7372

Merged

simaishi added the ivanchuk/backported label Oct 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native viewer #6574

Native viewer #6574

gekorob commented Dec 24, 2019 •

edited

Loading

gekorob commented Dec 24, 2019

skateman Dec 24, 2019

skateman Dec 24, 2019

gekorob Dec 24, 2019 •

edited

Loading

gekorob Dec 24, 2019

gekorob Dec 24, 2019

skateman Jan 3, 2020

gekorob Jan 3, 2020

skateman commented Dec 24, 2019

skateman commented Jan 3, 2020

gekorob commented Jan 3, 2020

gekorob commented Jan 3, 2020 •

edited

Loading

miq-bot commented Jan 7, 2020

gekorob commented Jan 7, 2020

gekorob commented Jan 7, 2020

skateman Jan 8, 2020

gekorob Jan 8, 2020

skateman Jan 8, 2020

skateman commented Jan 8, 2020

gekorob commented Jan 8, 2020

mzazrivec commented Jan 8, 2020

gekorob commented Jan 8, 2020

skateman commented Jan 8, 2020 •

edited

Loading

gekorob commented Jan 8, 2020

epwinchell commented Jan 8, 2020

martinpovolny Jan 8, 2020

gekorob Jan 8, 2020

gekorob commented Jan 8, 2020

epwinchell left a comment

skateman commented Jan 9, 2020

skateman left a comment

skateman commented Jan 11, 2020

simaishi commented Oct 6, 2020

Native viewer #6574

Native viewer #6574

Conversation

gekorob commented Dec 24, 2019 • edited Loading

gekorob commented Dec 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gekorob Dec 24, 2019 • 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

skateman commented Dec 24, 2019

skateman commented Jan 3, 2020

gekorob commented Jan 3, 2020

gekorob commented Jan 3, 2020 • edited Loading

miq-bot commented Jan 7, 2020

gekorob commented Jan 7, 2020

gekorob commented Jan 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skateman commented Jan 8, 2020

gekorob commented Jan 8, 2020

mzazrivec commented Jan 8, 2020

gekorob commented Jan 8, 2020

skateman commented Jan 8, 2020 • edited Loading

gekorob commented Jan 8, 2020

epwinchell commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gekorob commented Jan 8, 2020

epwinchell left a comment

Choose a reason for hiding this comment

skateman commented Jan 9, 2020

skateman left a comment

Choose a reason for hiding this comment

skateman commented Jan 11, 2020

simaishi commented Oct 6, 2020

gekorob commented Dec 24, 2019 •

edited

Loading

gekorob Dec 24, 2019 •

edited

Loading

gekorob commented Jan 3, 2020 •

edited

Loading

skateman commented Jan 8, 2020 •

edited

Loading