Open URL through automate for various objects #380

martinpovolny · 2019-06-11T07:05:29Z

Adding a table for temporary storing URL returned from automate.

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

UI PR: ManageIQ/manageiq-ui-classic#5688
Guides article: https://github.com/martinpovolny/guides/blob/automate_url_opener/automate_url_open.md (from PR: ManageIQ/guides#355)
Core PR: ManageIQ/manageiq#18849
Automate PR: ManageIQ/manageiq-automation_engine#328

miq-bot · 2019-06-11T07:12:31Z

Checked commit martinpovolny@8512e5a with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 1 offense detected

db/migrate/20160309144534_create_system_consoles.rb

❗ - Line 12, Col 7 - Layout/LeadingCommentSpace - Missing space after #.

martinpovolny · 2019-06-25T09:50:44Z

ping @Ladas

Ladas · 2019-06-25T11:58:00Z

db/migrate/20190604090631_create_saved_urls.rb

+  def change
+    create_table :saved_urls do |t|
+      t.bigint  "refers_to_id", :type => :bigint
+      t.string  "refers_to_type"


t.references :refers_to, :polymorphic => true, :index => true, :type => :bigint

should create the refers_to_id and ..._type` and the index

Ladas · 2019-06-25T12:09:21Z

db/migrate/20190604090631_create_saved_urls.rb

@@ -0,0 +1,12 @@
+class CreateSavedUrls < ActiveRecord::Migration[5.0]
+  def change
+    create_table :saved_urls do |t|


should be create_table :saved_urls, :id => :bigserial I believe

Ladas · 2019-06-25T12:10:58Z

cc @lpichler what's the rake command to generate the schema? We should still run it, right? (The CI was failing without it in the past)

martinpovolny · 2019-06-29T14:41:32Z

@Ladas : PR updated according to your guidance. Please, check again.

Ladas · 2019-07-01T08:06:48Z

db/migrate/20190604090631_create_external_urls.rb

+class CreateExternalUrls < ActiveRecord::Migration[5.0]
+  def change
+    create_table :external_urls, :id => :bigserial do |t|
+      t.references :refers_to, :polymorphic => true, :index => true, :type => :bigint


if the index => true here does't work, lets remove it (it should create external_urls for you, but it's not implemented in all migration methods)

refers_to maybe just resource ?

lpichler · 2019-07-01T11:59:24Z

db/migrate/20190604090631_create_external_urls.rb

+  def change
+    create_table :external_urls, :id => :bigserial do |t|
+      t.references :refers_to, :polymorphic => true, :index => true, :type => :bigint
+      t.string  "url"


nitpick: but we are using symbols:

t.string "url"

Agree with @lpichler here, can we use all symbols for the column names?

I missed that. Fixed now.

lpichler · 2019-07-01T11:59:38Z

db/migrate/20190604090631_create_external_urls.rb

+      t.belongs_to :user, :type => :bigint
+    end
+
+    add_index "external_urls", ["refers_to_id", "refers_to_type"], :name => "saved_urls_on_refers_to_id_and_refers_to_type"


lpichler · 2019-07-01T12:01:15Z

cc @lpichler what's the rake command to generate the schema? We should still run it, right? (The CI was failing without it in the past)

looks like it was removed: http://talk.manageiq.org/t/new-schema-specs-for-new-replication/1404/4

martinpovolny · 2019-07-01T12:18:48Z

@Ladas : fixed.

@lpichler : renamed and also renamed in the core PR here: ManageIQ/manageiq#18849

lpichler · 2019-07-01T12:21:10Z

@miq-bot assign @carbonin

carbonin · 2019-07-01T14:31:03Z

@martinpovolny I think the BZ link should be https://bugzilla.redhat.com/show_bug.cgi?id=1643331, right?

The one in the initial PR comment is closed.
Also could you add it to the commit message so the bot will pick it up and comment on the BZ?

carbonin · 2019-07-01T14:47:15Z

Also, what objects are going to be referenced by this url and if it's temporary, why do we need a table?

agrare · 2019-07-01T14:51:39Z

@martinpovolny If the goal is to show the vm summary page why a new table and not an attribute on the vm like we do with topology?

Also, the URL for vsphere is going to be different depending on version, if you have the html5 UI or just the old flash UI, etc... so I'm curious who would be generating this URL.

E.g. for vSphere 6.7 it looks like this https://dev-vc67/ui/#?extensionId=vsphere.core.inventory.serverObjectViewsExtension&objectId=urn:vmomi:VirtualMachine:vm-337:a6b3087a-3d6b-4852-9732-17094d702269&navigator=vsphere.core.viTree.vmsAndTemplatesView so I'm curious who would be generating this URL as it is highly provider dependent

martinpovolny · 2019-07-01T15:11:48Z

@carbonin : the link is correct.

@martinpovolny If the goal is to show the vm summary page why a new table and not an attribute on the vm like we do with topology?

No that is not the goal. The goal is in the BZ. If the goal is not clear from the BZ, then, please, read the (partial) documentation in this PR: ManageIQ/guides#355

This PR has nothing to do with the VM consoles although it can be used to replace a VM console with a custom one.

martinpovolny · 2019-07-01T15:13:57Z

Also, what objects are going to be referenced by this url and if it's temporary, why do we need a table?

How else do we pass data from a worker running on one appliance to a different appliance?

agrare · 2019-07-01T15:22:22Z

The bz you have linked to in the description is closed as a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=1643331

The goal of the BZ is pretty clear:

Description of request:

Ability to create CloudForms custom buttons which can open an HTTPS URL to a provider's web client.

Also important that this also isn't the VM's console, but the VM's summary page.

martinpovolny · 2019-07-01T15:40:50Z

@NickLaMuro, @agrare: this is about extending an existing mechanism (mentioned for example here:
http://talk.manageiq.org/t/lauch-ssh-error/4343/15 ) to work with several new types of entities.

I have implemented that mechanism several years ago, and at that time I did put the URL in the SystemConsole table. That limits the usefulnes of that to just VMs.

Here I am trying to do the same in a more universal way.

I do not know how else to explain it to you. Above I have linked a PR to a guides PR where the mechanism is explained: https://github.com/martinpovolny/guides/blob/automate_url_opener/automate_url_open.md.

Do you need any further explanation? Shall we schedule a video chat?

NickLaMuro · 2019-07-01T15:49:21Z

@martinpovolny Wrong "Nick" I think...

cc @carbonin

martinpovolny · 2019-07-01T16:04:20Z

The bz you have linked to in the description is closed as a duplicate of > https://bugzilla.redhat.com/show_bug.cgi?id=1643331

I have linked the one that contains a better explanation of the issue. On purpose.

You are linking the one that only mentiones one usecase and very shortly. Please, read the BZ that I linked.

agrare · 2019-07-01T16:39:29Z

Okay sounds like this bz https://bugzilla.redhat.com/show_bug.cgi?id=1643331 should have actually been marked as a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=1550002 then? We shouldn't be pointing PRs to closed BZs so lets either reopen the original one or add enough description to the current BZ.

martinpovolny · 2019-07-01T17:44:40Z

I am glad we moved forward ;-)

Are there any changes needed here on top of what @Ladas and @lpichler requested (and I already did)?

miq-bot added the wip label Jun 11, 2019

This was referenced Jun 11, 2019

Support for opening URL from the UI through automate. ManageIQ/manageiq-automation_engine#328

Merged

Support for opening URL from the UI through automate. ManageIQ/manageiq#18849

Merged

martinpovolny mentioned this pull request Jun 12, 2019

Opening URLs through automate ManageIQ/manageiq-ui-classic#5688

Merged

martinpovolny force-pushed the automate_url_open branch 2 times, most recently from 8d2a70f to 01f4348 Compare June 25, 2019 08:34

martinpovolny changed the title ~~[WIP] Open URL through automate for various objects~~ Open URL through automate for various objects Jun 25, 2019

Ladas reviewed Jun 25, 2019

View reviewed changes

martinpovolny force-pushed the automate_url_open branch from 01f4348 to 78de544 Compare June 29, 2019 14:40

Ladas reviewed Jul 1, 2019

View reviewed changes

lpichler reviewed Jul 1, 2019

View reviewed changes

martinpovolny force-pushed the automate_url_open branch from 78de544 to 96b077f Compare July 1, 2019 12:11

lpichler approved these changes Jul 1, 2019

View reviewed changes

miq-bot assigned carbonin Jul 1, 2019

Add table for storing URLs from automate.

3669477

martinpovolny force-pushed the automate_url_open branch from 96b077f to 3669477 Compare July 1, 2019 15:27

carbonin added changelog/yes enhancement schema and removed wip labels Jul 1, 2019

carbonin approved these changes Jul 1, 2019

View reviewed changes

carbonin merged commit 339c7fb into ManageIQ:master Jul 1, 2019

carbonin added this to the Sprint 115 Ending Jul 8, 2019 milestone Jul 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Open URL through automate for various objects #380

Open URL through automate for various objects #380

martinpovolny commented Jun 11, 2019 •

edited

Loading

miq-bot commented Jun 11, 2019

martinpovolny commented Jun 25, 2019

Ladas Jun 25, 2019 •

edited

Loading

Ladas Jun 25, 2019

Ladas commented Jun 25, 2019

martinpovolny commented Jun 29, 2019

Ladas Jul 1, 2019

lpichler Jul 1, 2019

lpichler Jul 1, 2019

carbonin Jul 1, 2019

martinpovolny Jul 1, 2019

lpichler Jul 1, 2019

lpichler commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

lpichler commented Jul 1, 2019

carbonin commented Jul 1, 2019

carbonin commented Jul 1, 2019

agrare commented Jul 1, 2019 •

edited

Loading

martinpovolny commented Jul 1, 2019 •

edited

Loading

martinpovolny commented Jul 1, 2019

agrare commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

NickLaMuro commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

agrare commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

Open URL through automate for various objects #380

Open URL through automate for various objects #380

Conversation

martinpovolny commented Jun 11, 2019 • edited Loading

miq-bot commented Jun 11, 2019

martinpovolny commented Jun 25, 2019

Ladas Jun 25, 2019 • edited Loading

Choose a reason for hiding this comment

Ladas Jun 25, 2019

Choose a reason for hiding this comment

Ladas commented Jun 25, 2019

martinpovolny commented Jun 29, 2019

Ladas Jul 1, 2019

Choose a reason for hiding this comment

lpichler Jul 1, 2019

Choose a reason for hiding this comment

lpichler Jul 1, 2019

Choose a reason for hiding this comment

carbonin Jul 1, 2019

Choose a reason for hiding this comment

martinpovolny Jul 1, 2019

Choose a reason for hiding this comment

lpichler Jul 1, 2019

Choose a reason for hiding this comment

lpichler commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

lpichler commented Jul 1, 2019

carbonin commented Jul 1, 2019

carbonin commented Jul 1, 2019

agrare commented Jul 1, 2019 • edited Loading

martinpovolny commented Jul 1, 2019 • edited Loading

martinpovolny commented Jul 1, 2019

agrare commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

NickLaMuro commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

agrare commented Jul 1, 2019

martinpovolny commented Jul 1, 2019

martinpovolny commented Jun 11, 2019 •

edited

Loading

Ladas Jun 25, 2019 •

edited

Loading

agrare commented Jul 1, 2019 •

edited

Loading

martinpovolny commented Jul 1, 2019 •

edited

Loading