Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Migration Details Page Should Auto-Refresh #311

Closed
kedark3 opened this issue May 16, 2018 · 47 comments · Fixed by #339
Closed

Migration Details Page Should Auto-Refresh #311

kedark3 opened this issue May 16, 2018 · 47 comments · Fixed by #339

Comments

@kedark3
Copy link

kedark3 commented May 16, 2018

https://bugzilla.redhat.com/show_bug.cgi?id=1573625
Since we have a spinner on the page which gives false impression of page will be automatically refreshed, but it does not.

@priley86
Copy link
Member

@kedark3 can you provide some environment details here? Maybe you can follow up w/ @AparnaKarve and we can reproduce your issue in a lab. It is difficult to simulate this locally a.t.m.

@priley86 priley86 added the bug label May 16, 2018
@kedark3
Copy link
Author

kedark3 commented May 16, 2018

@priley86 @AparnaKarve I am happy to collaborate. Let me know what you need. 😃

@serenamarie125
Copy link

@mturley Can you take a look at this next?

@serenamarie125
Copy link

First step is to reproduce this in Brett's environment. If reproducible, let's fix it! If not, we have to wait for a response from QE in the BZ listed above.

@mturley
Copy link
Contributor

mturley commented May 17, 2018

Grabbing this as soon as #322 is merged.

@mturley mturley self-assigned this May 17, 2018
@serenamarie125
Copy link

Here's the image attached to the gif

peek 2018-05-17 17-08

@serenamarie125
Copy link

@vconzola @priley86 it is a Failed Migration Plan that is being looked at. I asked for some clarification.

No auto refresh is necessary because this is a failed migration plan, and the migration of the VM has already failed.

Looking at the details page, there is a utilization bar, which behaves like a progress bar. I'm wondering if this is the issue, from the reporters perspective. It does look like it's still in progress.

I've asked for more information, so let' see what he has to say.

@kedark3
Copy link
Author

kedark3 commented May 18, 2018

@serena,
I agree that this is a failed migration. But if you navigate to same screen while migration is in progress, the utilization bar does not update dynamically.You need to hit refresh manually to see progress in the bar, as well as the clock for elapsed time increase. This all should update without needing to hit refresh on the page.
Hope that helps.

@serenamarie125
Copy link

ahhh ok @kedark3 ! Thank you for the info.

@mturley please try and reproduce this when looking at the details of a migration in progress. Sit on that screen, and please verify that the utilization bar is not dynamically updating. If you cannot reproduce on your local environment, please use the environment Brett gave us access to. Please add a note to this Issue once you have tried to reproduced the issue

@mturley
Copy link
Contributor

mturley commented May 20, 2018

Thanks for the clarification @kedark3 and for the mention @serenamarie125. I tried to reproduce once and didn't see the issue, I'll take another look on Monday morning with this in mind.

@kedark3
Copy link
Author

kedark3 commented May 21, 2018

peek 2018-05-21 11-12
@mturley @serenamarie125 Please review this GIF, I tried to reproduce it this morning. I cannot see Time/Progress bar updating dynamically. I can provide appliance IP if you need.

@mturley
Copy link
Contributor

mturley commented May 21, 2018

@kedark3 I agree that the timer should update in real-time, even though the polling is on every 15 seconds. I'm trying to reproduce the issue now, and I think we will add that real-time elapsing to the timer, but does the progress bar not update for you after waiting longer than 15 seconds? Checking on your environment right now myself as well.

@kedark3
Copy link
Author

kedark3 commented May 21, 2018

@mturley Yes Mike, the progress bar does not update. Even if I wait.

@JPrause
Copy link
Member

JPrause commented May 21, 2018

This is a blocker for issue: https://bugzilla.redhat.com/show_bug.cgi?id=1573625

@mturley
Copy link
Contributor

mturley commented May 21, 2018

@kedark3 it appears to update for me if I wait long enough. What browser are you using? do you see any errors in your JS console?

http://recordit.co/Mr9HlkXe7a <-- see about 30 seconds into this video. Sorry, I navigated away before the first 15 seconds resolved.

@kedark3
Copy link
Author

kedark3 commented May 21, 2018

@mturley you should see Migration Details Page which is https://10.8.196.145/migration/plan/6, it does not update

@mturley
Copy link
Contributor

mturley commented May 21, 2018

Sorry for confusion, yes, that page appears to have a bug. I have a potential fix, I'm trying to figure out how best to test it. We may just ship it and see-- i think it's a harmless change, @priley86 can you look at this for me? #339

I think if that PR doesn't fix it, we have a remounting issue or some other more serious race condition.

@mturley
Copy link
Contributor

mturley commented May 21, 2018

@AparnaKarve mentioned she is making some time for this issue later today. Aparna maybe you can pick up where i left off with #339 or help me test it

@kedark3
Copy link
Author

kedark3 commented May 22, 2018

@mturley If you want me to test it, including it in nightly build will be most reliable and easy way for testing it.

@mturley
Copy link
Contributor

mturley commented May 22, 2018

That's what I figured, thanks @kedark3. We might do just that, i'll keep you posted.

@mturley
Copy link
Contributor

mturley commented May 22, 2018

@priley86 I think your closing of my PR auto-closed this because I mentioned it with the magic word "fix". Reopening until @kedark3 has a chance to test it. This will be in the nightly, @kedark3, let us know if it solves the problem for you!

@mturley mturley reopened this May 22, 2018
@kedark3
Copy link
Author

kedark3 commented May 23, 2018

@mturley I will update you on this when nightly becomes available. Nightly from yesterday is not available as it seems.

@mturley
Copy link
Contributor

mturley commented May 23, 2018

@priley86 how can we check on the nightlies for this? Maybe moving the repo broke it?

@priley86
Copy link
Member

will wait on @bthurber to confirm this is available for QA - should be g2g for nightly
ManageIQ/manageiq#17442

@bthurber
Copy link

Deployed last evening's nightly and not seeing dynamic updates in the migration plan details. Appears to still be broken.

@priley86
Copy link
Member

Ok thanks @bthurber. Will continue investigating if we can login to the MIQ instance there. Apologies...

@JPrause
Copy link
Member

JPrause commented May 24, 2018

@miq-bot add_label blocker

@mturley
Copy link
Contributor

mturley commented May 24, 2018

That's disconcerting. @priley86 I could use help debugging this one if it's not solved by that fix. Can we confirm that the PR made it into the nightly they used?

@AparnaKarve
Copy link
Contributor

@mturley I will be trying the following change on Brett's machine shortly...

this.startPolling(planRequestId);
if (mostRecentRequest.fulfilled_on !== null) {
  this.stopPolling();
  this.setState(() => ({
  planFinished: true
 }));
}

@AparnaKarve
Copy link
Contributor

AparnaKarve commented May 24, 2018

@mturley @priley86 News flash!!
Just looked into the code on Brett's machine and it still shows the old code -

if (mostRecentRequest.status === 'active') {
          this.startPolling(planRequestId);
        } else if (
          mostRecentRequest.request_state === 'finished' ||
          mostRecentRequest.status === 'Error' 
        ) {     
          this.setState(() => ({
            planFinished: true
          }));    
        } 

It looks like our changes in this area never made it to the nightly build.

However, I do think that the above code change #311 (comment) is necessary.

Also noticed that the Gemfile is still pointing to -
"https://github.com/priley86/miq_v2v_ui_plugin.git", :branch => "alpha"

@mturley
Copy link
Contributor

mturley commented May 24, 2018

Thanks @AparnaKarve . If you need to work on anything else let me know and I'll take over again. I want to make sure we get this fixed asap.

@mturley
Copy link
Contributor

mturley commented May 24, 2018

Hmm.... I wonder why that is.... I thought I had updated everything. I'll take another look.

@mturley
Copy link
Contributor

mturley commented May 24, 2018

@AparnaKarve I think the code change we did check in will fix the issue, but I agree yours seems more reliable. Are you making another PR? I'll look into why my old gemfile is still in there.

@AparnaKarve
Copy link
Contributor

I'll look into why my old gemfile is still in there.

@mturley I was referring to the Gemfile that is used in the nightly builds - it looks like it is still using the old repo (at least, that is what I saw on Brett's machine)

I will make a PR after I actually test and verify the changes on Brett's machine

@mturley
Copy link
Contributor

mturley commented May 24, 2018

Ahh, that makes more sense. I thought you had seen that in my PR or I had otherwise missed s something.

Thanks again @AparnaKarve let me know if there is anything I can do to help.

@AparnaKarve
Copy link
Contributor

@mturley I have confirmed on the appliance that the page refreshes correctly with the latest master.

I don't think we need an additional PR to do this - #311 (comment)

There is one thing missing though on the Plan detail page -- the notification popup when the migration completes, for which I can make a PR -- should be fairly simple I think.

@kedark3
Copy link
Author

kedark3 commented May 25, 2018

@AparnaKarve is it already in nightly? if so I can try to validate it.

@AparnaKarve
Copy link
Contributor

@kedark3 I don't know if the nightly build issue is fixed or not.

On your appliance, can you check the "About" box and see what MIQ version and date it shows?

@kedark3
Copy link
Author

kedark3 commented May 25, 2018

Its still old. :(

@JPrause
Copy link
Member

JPrause commented May 29, 2018

@AllenBW can you add the blocker label for me,...there's a nickel in it for you.

@serenamarie125
Copy link

@bthurber fyi - we are still waiting for this to be tested in nightly. Just want you to be aware.

@bthurber
Copy link

Verified this is fixed in latest nightly.

@serenamarie125
Copy link

Fixed by #339 🎉🍻

@mturley
Copy link
Contributor

mturley commented May 30, 2018

Wooo! Not bad to find out I fixed it properly 8 days ago. The suspense was killing me. Thanks @bthurber

@bthurber
Copy link

@mturley confirmed this is not fixed in current 5.9.3.1 downstream build. Need to verify commit is pushed there so we can unblock the next 5.9.3.1 build.

@mturley
Copy link
Contributor

mturley commented Jun 18, 2018

Hmm. Ok. It was fixed in the nightly, though? Not sure how to check the downstream build, how can I help?

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

Successfully merging a pull request may close this issue.

8 participants