Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

[#499] handle missing clusters, networks, storages #503

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

priley86
Copy link
Member

@priley86 priley86 commented Jul 19, 2018

fixes #499
fixes https://bugzilla.redhat.com/show_bug.cgi?id=1602796

  • Safeguard against deleted networks, datastorages, clusters in the infrastructure mapping list view.
  • Ensure the UI will not break due to null references / missing values.
  • Display "Clusters/Networks/Datastores missing" errors in the list.
  • For simplicity, if there are any missing clusters, display missing data for networks/storages as well.

Tested this against the QE database w/ the missing LAN, and it will now display the following:
screen shot 2018-07-19 at 11 05 55 am

Here's how it would look if all were missing though:
screen shot 2018-07-19 at 11 09 06 am

It seems the original issue was that a provider was deleted. I will see how this checks out w/ that scenario shortly...

Update: testing this against the other scenario, which is deleting the RHV provider, and then adding it back. The UI will no longer crash and show the messaging above. Works as expected.

  • Note: I personally think it is far too much additional logic to do all these checks in the Plan Wizard (and would just convolute the UI). In the future, the backend might flag these records for us and make that easier. For now, we will have to let the user run a migration which will likely fail. Still need to validate this scenario w/ QE, but the initial attempt here is to just prevent the UI from becoming unresponsive.

@priley86 priley86 added bug bz Issues filed by QE or having a BZ Sprint 11 labels Jul 19, 2018
@AparnaKarve
Copy link
Contributor

I personally think it is far too much additional logic to do all these checks in the Plan Wizard

Yes, agree.

@priley86 Tested this in the UI. Changes look good and prevent the UI from breaking.
You have covered all possible cases in this PR - clusters, datastores and lans, so I think the crash is well addressed for any of the "missing id" use cases.

Can I request one minor change?
Can you remove the period in these strings --
Networks missing./ Datastores missing. / Clusters missing.

@priley86
Copy link
Member Author

good point - they are inconsistent with other labels, and PF does not include them...removing them now.

@priley86 priley86 changed the title [WIP] [#499] handle missing clusters, networks, storages [#499] handle missing clusters, networks, storages Jul 19, 2018
@priley86 priley86 requested a review from michaelkro July 19, 2018 20:37
@AparnaKarve AparnaKarve self-requested a review July 19, 2018 21:12
Copy link
Contributor

@AparnaKarve AparnaKarve left a comment

Choose a reason for hiding this comment

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

Works well, looks good!

Copy link
Contributor

@michaelkro michaelkro left a comment

Choose a reason for hiding this comment

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

Also tested using the QE database, works as expected 🎊

@AparnaKarve AparnaKarve merged commit 85699b7 into ManageIQ:master Jul 19, 2018
@kedark3
Copy link

kedark3 commented Jul 20, 2018

@priley86 we don't have 19th July nightly yet, if we have it today, we can test it or on Monday.

@kedark3
Copy link

kedark3 commented Jul 30, 2018

5.9.4.1 is still broken,https://bugzilla.redhat.com/show_bug.cgi?id=1602796#c9

Although, 5.10.0.6 is showing correctly as mentioned in #503 (comment)

@AparnaKarve
Copy link
Contributor

This has not been backported to G yet, and hence 5.9.4.1 would appear to be broken.

simaishi pushed a commit that referenced this pull request Jul 31, 2018
[#499] handle missing clusters, networks, storages
(cherry picked from commit 85699b7)

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

Gaprindashvili backport details:

$ git log -1
commit d7f9ab73a9bbe6abca8c46a7ab364e4841c5d80d
Author: Aparna Karve <aparna.karve@gmail.com>
Date:   Thu Jul 19 14:44:21 2018 -0700

    Merge pull request #503 from priley86/deleted-network
    
    [#499] handle missing clusters, networks, storages
    (cherry picked from commit 85699b77ecd60388760424974e6821ad24ceaf44)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1610077

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V2V Migration UI becomes unusable on provider add-remove-add
5 participants